Re: [ZEND-ENGINE-CVS] cvs: TSRM / tsrm_virtual_cwd.h
| From: | Jason T. Greene | 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>