Re: [ZEND-ENGINE-CVS] cvs: TSRM / tsrm_virtual_cwd.h

From: Date: Fri, 13 Dec 2002 15:12:14 +0000
Subject: Re: [ZEND-ENGINE-CVS] cvs: TSRM / tsrm_virtual_cwd.h
References: 1 2 3  Groups: php.zend-engine.cvs 
Request: Send a blank email to php-zend-engine-cvs+get-714@lists.php.net to get a copy of this message
A thorough comment explaining the logic in solving this issue would help, even if it seems to obvious to the developer writing the code. -Jason On Wed, 2002-12-11 at 23:40, Andi Gutmans wrote: > At 02:06 AM 12/11/2002 -0500, Ilia A. wrote: > >On December 11, 2002 12:16 am, Andi Gutmans wrote: > > > This is not the correct patch but adds other bugs. You should check for UNC > > > separetely in the code like in virtual_realpath(). IS_ABSOLUTE_PATH() is > > > only meant to detect the drive letter and is always followed by use of > > > COPY_WHEN_ABSOLUTE() (which happens to also be 2 but it's still not right. > > > Stuff might break). > > > >The patch only affects the behavior of IS_ABSOLUTE_PATH on Win32 where > >UNC path is absolute. PHP's behavior is the same when either IS_ABSOLUTE_PATH > >or IS_UNC_PATH occur. Separation of the two, only adds another len >= 2 > >check, beyond that nothing else changes. I do not see how this can break any > >existing code on Win32 platform, perhaps you could offer me some hints about > >situations where this may cause problems? > >I've looked at the usage of IS_ABSOLUTE_PATH macro and it is only used 4 > >times > >outside of TSRM/tsrm_virtual_cwd.c, where it only used in 2 functions. From > >what I've been able to see the change does not break any existing code. It > >can however prevent problems for paths on Win32 that begin with \\ or //, > >since unless the UNC check is explicitly done the code will try to 'resolve' > >those paths breaking them in the process. > > I didn't mean it'll break things right away. I'm just saying the fact that > by chance sizeof("c:") == sizeof("\\") should not be taken advantage of by > the code (IMO). It means the code doesn't stay very clean and if we ever > make a change there things might break. > I think we should change COPY_WHEN_ABSOLUTE(path) to > patch_copy[1] == ':' ? 2 : 2 > > and then we'll be ready for making future changes and it'll be obvious how > it works to people reading the code instead of relying on a coincidence. > The compiler will even optimize this (most probably). > > Andi -- Jason T. Greene <jason@php.net>

« previous php.zend-engine.cvs (#714) next »