You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by Bert Huijben <be...@qqmail.nl> on 2010/01/19 12:19:28 UTC

[Patch] RE: Windows drive letter check fails on lower case current working drive

	Ping,

This issue (and mailinglist thread) is now more than two months old. The latest patch adds a testcase showing the issue in the APR testsuite. (There is also an XFail test in the subversion testsuite for this issue)

The patch was written on the 1.4.x branch but I svn switch'ed it to trunk for easy application.

	Bert

> -----Original Message-----
> From: Bert Huijben [mailto:bert@qqmail.nl]
> Sent: woensdag 6 januari 2010 13:45
> To: 'William A. Rowe, Jr.'; dev@apr.apache.org
> Cc: 'Philip Martin'; 'Bert Huijben'; dev@subversion.tigris.org
> Subject: RE: Windows drive letter check fails on lower case cwd
> 
> 
> 
> > -----Original Message-----
> > From: Bert Huijben [mailto:bert@qqmail.nl]
> > Sent: maandag 9 november 2009 10:05
> > To: 'William A. Rowe, Jr.'
> > Cc: 'Philip Martin'; 'Bert Huijben'; dev@apr.apache.org;
> > dev@subversion.tigris.org
> > Subject: RE: Windows drive letter check fails on lower case cwd
> >
> > > -----Original Message-----
> > > From: William A. Rowe, Jr. [mailto:wrowe@rowe-clan.net]
> > > Sent: maandag 9 november 2009 4:25
> > > To: Bert Huijben
> > > Cc: 'Philip Martin'; 'Bert Huijben'; dev@apr.apache.org;
> > > dev@subversion.tigris.org
> > > Subject: Re: Windows drive letter check fails on lower case cwd
> > >
> > > Bert Huijben wrote:
> > > >
> > > > The drive letters don't have locales; the rest of the paths have.
> > > There are
> > > > only 26 driveletters with the US-ASCII characters A-Z. (Internally
> > > always
> > > > represented by the upper case letters, but the current path can use a
> > > lower
> > > > case path, as that is only managed in userspace).
> > >
> > > That's an odd statement; all paths are Unicode ;-)  We don't strcasecmp
> > > paths.
> >
> > s/path/drive in its path/ for that last path.. thanks :)
> >
> > Paths are (of course) unicode, but use a some culture sensitive compare to
> > handle the case insensitivity. But there is no documented way to find which
> > locale it uses for each (part of a) drive.
> > (That information is stored in the system portion of NTFS at format time
> and
> > can vary over directories via junctions, etc.)
> >
> > Thanks for looking into this.
> 
> After some delay I created a new patch with the suggested changes and a
> testcase for inclusion in the apr testsuite. (Note that the test can falsely
> succeed if your test environment is on C:)
> 
> I tested this patch on the 1.4.x branch and 1.3.x, but created the patch based
> on trunk. (The visual studio projects don't include the expat build, but it is
> required for a successful compilation).
> 
> As noted earlier, this issue will be experienced by far more users once
> Subversion 1.7 is released, so it would be nice if it is backported to the
> relevant releases.
> 
> 	Bert

RE: [Patch] RE: Windows drive letter check fails on lower case current working drive

Posted by Bert Huijben <be...@qqmail.nl>.

> -----Original Message-----
> From: Bert Huijben [mailto:bert@qqmail.nl]
> Sent: woensdag 20 januari 2010 11:31
> To: 'Branko Čibej'; 'William A. Rowe Jr.'
> Cc: dev@apr.apache.org; 'Philip Martin'; 'Bert Huijben';
> dev@subversion.apache.org
> Subject: RE: [Patch] RE: Windows drive letter check fails on lower case
> current working drive
> 
> > -----Original Message-----
> > From: Branko Čibej [mailto:brane@xbc.nu]
> > Sent: woensdag 20 januari 2010 10:17
> > To: William A. Rowe Jr.
> > Cc: Bert Huijben; dev@apr.apache.org; 'Philip Martin'; 'Bert
> Huijben';
> > dev@subversion.apache.org
> > Subject: Re: [Patch] RE: Windows drive letter check fails on lower
> case
> > current working drive
> >
> > William A. Rowe Jr. wrote:
> > > On 1/19/2010 5:19 AM, Bert Huijben wrote:
> > >
> > >> The patch was written on the 1.4.x branch but I svn switch'ed it
> to trunk
> > for easy application.
> > >>
> > >
> > > I would suggest one bit of alternate code that is a bit more
> condensed, any
> > > concern?;
> > >
> > > +static int same_drive(const char *path1, const char *path2)
> > > +{
> > > +    if ((path1[0] < 'A' || (path1[0] > 'Z' && path1[0] < 'a') ||
> path1[0] > 'Z')
> > > +     || (path2[0] < 'A' || (path2[0] > 'Z' && path2[0] < 'a') ||
> path2[0] > 'Z')
> > > +     || path1[1] != ':' || path2[1] != ':')
> > > +        return 0;
> > >
> > > +    /* Once in the domain of ASCII alpha, compare these case
> insensitive */
> > > +    return ((path1[0] & 0x1f) == (path2[0] & 0x1f));
> > > +}
> > >
> > Whatever you do, those last Z's should be z's or it all breaks.
> 
> No, problem... I initially wrote a more condensed form myself and just
> made it a bit more readable later.
> 
> But please fix that 'Z' ;)

	Ping,

Subversion is trying to get Subversion 1.7 test releases out by this summer, and we really need this fixed (or a workaround) before this release.

	Bert



RE: [Patch] RE: Windows drive letter check fails on lower case current working drive

Posted by Bert Huijben <be...@qqmail.nl>.

> -----Original Message-----
> From: Bert Huijben [mailto:bert@qqmail.nl]
> Sent: woensdag 20 januari 2010 11:31
> To: 'Branko Čibej'; 'William A. Rowe Jr.'
> Cc: dev@apr.apache.org; 'Philip Martin'; 'Bert Huijben';
> dev@subversion.apache.org
> Subject: RE: [Patch] RE: Windows drive letter check fails on lower case
> current working drive
> 
> > -----Original Message-----
> > From: Branko Čibej [mailto:brane@xbc.nu]
> > Sent: woensdag 20 januari 2010 10:17
> > To: William A. Rowe Jr.
> > Cc: Bert Huijben; dev@apr.apache.org; 'Philip Martin'; 'Bert
> Huijben';
> > dev@subversion.apache.org
> > Subject: Re: [Patch] RE: Windows drive letter check fails on lower
> case
> > current working drive
> >
> > William A. Rowe Jr. wrote:
> > > On 1/19/2010 5:19 AM, Bert Huijben wrote:
> > >
> > >> The patch was written on the 1.4.x branch but I svn switch'ed it
> to trunk
> > for easy application.
> > >>
> > >
> > > I would suggest one bit of alternate code that is a bit more
> condensed, any
> > > concern?;
> > >
> > > +static int same_drive(const char *path1, const char *path2)
> > > +{
> > > +    if ((path1[0] < 'A' || (path1[0] > 'Z' && path1[0] < 'a') ||
> path1[0] > 'Z')
> > > +     || (path2[0] < 'A' || (path2[0] > 'Z' && path2[0] < 'a') ||
> path2[0] > 'Z')
> > > +     || path1[1] != ':' || path2[1] != ':')
> > > +        return 0;
> > >
> > > +    /* Once in the domain of ASCII alpha, compare these case
> insensitive */
> > > +    return ((path1[0] & 0x1f) == (path2[0] & 0x1f));
> > > +}
> > >
> > Whatever you do, those last Z's should be z's or it all breaks.
> 
> No, problem... I initially wrote a more condensed form myself and just
> made it a bit more readable later.
> 
> But please fix that 'Z' ;)

	Ping,

Subversion is trying to get Subversion 1.7 test releases out by this summer, and we really need this fixed (or a workaround) before this release.

	Bert



RE: [Patch] RE: Windows drive letter check fails on lower case current working drive

Posted by Bert Huijben <be...@qqmail.nl>.

> -----Original Message-----
> From: Bert Huijben [mailto:bert@qqmail.nl]
> Sent: woensdag 20 januari 2010 11:31
> To: 'Branko Čibej'; 'William A. Rowe Jr.'
> Cc: dev@apr.apache.org; 'Philip Martin'; 'Bert Huijben';
> dev@subversion.apache.org
> Subject: RE: [Patch] RE: Windows drive letter check fails on lower case
> current working drive
> 
> > -----Original Message-----
> > From: Branko Čibej [mailto:brane@xbc.nu]
> > Sent: woensdag 20 januari 2010 10:17
> > To: William A. Rowe Jr.
> > Cc: Bert Huijben; dev@apr.apache.org; 'Philip Martin'; 'Bert
> Huijben';
> > dev@subversion.apache.org
> > Subject: Re: [Patch] RE: Windows drive letter check fails on lower
> case
> > current working drive
> >
> > William A. Rowe Jr. wrote:
> > > On 1/19/2010 5:19 AM, Bert Huijben wrote:
> > >
> > >> The patch was written on the 1.4.x branch but I svn switch'ed it
> to trunk
> > for easy application.
> > >>
> > >
> > > I would suggest one bit of alternate code that is a bit more
> condensed, any
> > > concern?;
> > >
> > > +static int same_drive(const char *path1, const char *path2)
> > > +{
> > > +    if ((path1[0] < 'A' || (path1[0] > 'Z' && path1[0] < 'a') ||
> path1[0] > 'Z')
> > > +     || (path2[0] < 'A' || (path2[0] > 'Z' && path2[0] < 'a') ||
> path2[0] > 'Z')
> > > +     || path1[1] != ':' || path2[1] != ':')
> > > +        return 0;
> > >
> > > +    /* Once in the domain of ASCII alpha, compare these case
> insensitive */
> > > +    return ((path1[0] & 0x1f) == (path2[0] & 0x1f));
> > > +}
> > >
> > Whatever you do, those last Z's should be z's or it all breaks.
> 
> No, problem... I initially wrote a more condensed form myself and just
> made it a bit more readable later.
> 
> But please fix that 'Z' ;)

	Ping,

Subversion is trying to get Subversion 1.7 test releases out by this summer, and we really need this fixed (or a workaround) before this release.

	Bert


RE: [Patch] RE: Windows drive letter check fails on lower case current working drive

Posted by Bert Huijben <be...@qqmail.nl>.
> -----Original Message-----
> From: Branko Čibej [mailto:brane@xbc.nu]
> Sent: woensdag 20 januari 2010 10:17
> To: William A. Rowe Jr.
> Cc: Bert Huijben; dev@apr.apache.org; 'Philip Martin'; 'Bert Huijben';
> dev@subversion.apache.org
> Subject: Re: [Patch] RE: Windows drive letter check fails on lower case
> current working drive
> 
> William A. Rowe Jr. wrote:
> > On 1/19/2010 5:19 AM, Bert Huijben wrote:
> >
> >> The patch was written on the 1.4.x branch but I svn switch'ed it to trunk
> for easy application.
> >>
> >
> > I would suggest one bit of alternate code that is a bit more condensed, any
> > concern?;
> >
> > +static int same_drive(const char *path1, const char *path2)
> > +{
> > +    if ((path1[0] < 'A' || (path1[0] > 'Z' && path1[0] < 'a') || path1[0] > 'Z')
> > +     || (path2[0] < 'A' || (path2[0] > 'Z' && path2[0] < 'a') || path2[0] > 'Z')
> > +     || path1[1] != ':' || path2[1] != ':')
> > +        return 0;
> >
> > +    /* Once in the domain of ASCII alpha, compare these case insensitive */
> > +    return ((path1[0] & 0x1f) == (path2[0] & 0x1f));
> > +}
> >
> Whatever you do, those last Z's should be z's or it all breaks.

No, problem... I initially wrote a more condensed form myself and just made it a bit more readable later.

But please fix that 'Z' ;)

Thanks,
	Bert

RE: [Patch] RE: Windows drive letter check fails on lower case current working drive

Posted by Bert Huijben <be...@qqmail.nl>.
> -----Original Message-----
> From: Branko Čibej [mailto:brane@xbc.nu]
> Sent: woensdag 20 januari 2010 10:17
> To: William A. Rowe Jr.
> Cc: Bert Huijben; dev@apr.apache.org; 'Philip Martin'; 'Bert Huijben';
> dev@subversion.apache.org
> Subject: Re: [Patch] RE: Windows drive letter check fails on lower case
> current working drive
> 
> William A. Rowe Jr. wrote:
> > On 1/19/2010 5:19 AM, Bert Huijben wrote:
> >
> >> The patch was written on the 1.4.x branch but I svn switch'ed it to trunk
> for easy application.
> >>
> >
> > I would suggest one bit of alternate code that is a bit more condensed, any
> > concern?;
> >
> > +static int same_drive(const char *path1, const char *path2)
> > +{
> > +    if ((path1[0] < 'A' || (path1[0] > 'Z' && path1[0] < 'a') || path1[0] > 'Z')
> > +     || (path2[0] < 'A' || (path2[0] > 'Z' && path2[0] < 'a') || path2[0] > 'Z')
> > +     || path1[1] != ':' || path2[1] != ':')
> > +        return 0;
> >
> > +    /* Once in the domain of ASCII alpha, compare these case insensitive */
> > +    return ((path1[0] & 0x1f) == (path2[0] & 0x1f));
> > +}
> >
> Whatever you do, those last Z's should be z's or it all breaks.

No, problem... I initially wrote a more condensed form myself and just made it a bit more readable later.

But please fix that 'Z' ;)

Thanks,
	Bert


RE: [Patch] RE: Windows drive letter check fails on lower case current working drive

Posted by Bert Huijben <be...@qqmail.nl>.
> -----Original Message-----
> From: Branko Čibej [mailto:brane@xbc.nu]
> Sent: woensdag 20 januari 2010 10:17
> To: William A. Rowe Jr.
> Cc: Bert Huijben; dev@apr.apache.org; 'Philip Martin'; 'Bert Huijben';
> dev@subversion.apache.org
> Subject: Re: [Patch] RE: Windows drive letter check fails on lower case
> current working drive
> 
> William A. Rowe Jr. wrote:
> > On 1/19/2010 5:19 AM, Bert Huijben wrote:
> >
> >> The patch was written on the 1.4.x branch but I svn switch'ed it to trunk
> for easy application.
> >>
> >
> > I would suggest one bit of alternate code that is a bit more condensed, any
> > concern?;
> >
> > +static int same_drive(const char *path1, const char *path2)
> > +{
> > +    if ((path1[0] < 'A' || (path1[0] > 'Z' && path1[0] < 'a') || path1[0] > 'Z')
> > +     || (path2[0] < 'A' || (path2[0] > 'Z' && path2[0] < 'a') || path2[0] > 'Z')
> > +     || path1[1] != ':' || path2[1] != ':')
> > +        return 0;
> >
> > +    /* Once in the domain of ASCII alpha, compare these case insensitive */
> > +    return ((path1[0] & 0x1f) == (path2[0] & 0x1f));
> > +}
> >
> Whatever you do, those last Z's should be z's or it all breaks.

No, problem... I initially wrote a more condensed form myself and just made it a bit more readable later.

But please fix that 'Z' ;)

Thanks,
	Bert


Re: [Patch] RE: Windows drive letter check fails on lower case current working drive

Posted by Branko Čibej <br...@xbc.nu>.
William A. Rowe Jr. wrote:
> On 1/19/2010 5:19 AM, Bert Huijben wrote:
>   
>> The patch was written on the 1.4.x branch but I svn switch'ed it to trunk for easy application.
>>     
>
> I would suggest one bit of alternate code that is a bit more condensed, any
> concern?;
>
> +static int same_drive(const char *path1, const char *path2)
> +{
> +    if ((path1[0] < 'A' || (path1[0] > 'Z' && path1[0] < 'a') || path1[0] > 'Z')
> +     || (path2[0] < 'A' || (path2[0] > 'Z' && path2[0] < 'a') || path2[0] > 'Z')
> +     || path1[1] != ':' || path2[1] != ':')
> +        return 0;
>
> +    /* Once in the domain of ASCII alpha, compare these case insensitive */
> +    return ((path1[0] & 0x1f) == (path2[0] & 0x1f));
> +}
>   
Whatever you do, those last Z's should be z's or it all breaks.

-- Brane

Re: [Patch] RE: Windows drive letter check fails on lower case current working drive

Posted by Branko Čibej <br...@xbc.nu>.
William A. Rowe Jr. wrote:
> On 1/19/2010 5:19 AM, Bert Huijben wrote:
>   
>> The patch was written on the 1.4.x branch but I svn switch'ed it to trunk for easy application.
>>     
>
> I would suggest one bit of alternate code that is a bit more condensed, any
> concern?;
>
> +static int same_drive(const char *path1, const char *path2)
> +{
> +    if ((path1[0] < 'A' || (path1[0] > 'Z' && path1[0] < 'a') || path1[0] > 'Z')
> +     || (path2[0] < 'A' || (path2[0] > 'Z' && path2[0] < 'a') || path2[0] > 'Z')
> +     || path1[1] != ':' || path2[1] != ':')
> +        return 0;
>
> +    /* Once in the domain of ASCII alpha, compare these case insensitive */
> +    return ((path1[0] & 0x1f) == (path2[0] & 0x1f));
> +}
>   
Whatever you do, those last Z's should be z's or it all breaks.

-- Brane

Re: [Patch] RE: Windows drive letter check fails on lower case current working drive

Posted by Branko Čibej <br...@xbc.nu>.
William A. Rowe Jr. wrote:
> On 1/19/2010 5:19 AM, Bert Huijben wrote:
>   
>> The patch was written on the 1.4.x branch but I svn switch'ed it to trunk for easy application.
>>     
>
> I would suggest one bit of alternate code that is a bit more condensed, any
> concern?;
>
> +static int same_drive(const char *path1, const char *path2)
> +{
> +    if ((path1[0] < 'A' || (path1[0] > 'Z' && path1[0] < 'a') || path1[0] > 'Z')
> +     || (path2[0] < 'A' || (path2[0] > 'Z' && path2[0] < 'a') || path2[0] > 'Z')
> +     || path1[1] != ':' || path2[1] != ':')
> +        return 0;
>
> +    /* Once in the domain of ASCII alpha, compare these case insensitive */
> +    return ((path1[0] & 0x1f) == (path2[0] & 0x1f));
> +}
>   
Whatever you do, those last Z's should be z's or it all breaks.

-- Brane

Re: [Patch] RE: Windows drive letter check fails on lower case current working drive

Posted by "William A. Rowe Jr." <wr...@rowe-clan.net>.
On 1/19/2010 5:19 AM, Bert Huijben wrote:
> 
> The patch was written on the 1.4.x branch but I svn switch'ed it to trunk for easy application.

I would suggest one bit of alternate code that is a bit more condensed, any
concern?;

+static int same_drive(const char *path1, const char *path2)
+{
+    if ((path1[0] < 'A' || (path1[0] > 'Z' && path1[0] < 'a') || path1[0] > 'Z')
+     || (path2[0] < 'A' || (path2[0] > 'Z' && path2[0] < 'a') || path2[0] > 'Z')
+     || path1[1] != ':' || path2[1] != ':')
+        return 0;

+    /* Once in the domain of ASCII alpha, compare these case insensitive */
+    return ((path1[0] & 0x1f) == (path2[0] & 0x1f));
+}

Re: [Patch] RE: Windows drive letter check fails on lower case current working drive

Posted by "William A. Rowe Jr." <wr...@rowe-clan.net>.
On 1/19/2010 5:19 AM, Bert Huijben wrote:
> 
> The patch was written on the 1.4.x branch but I svn switch'ed it to trunk for easy application.

I would suggest one bit of alternate code that is a bit more condensed, any
concern?;

+static int same_drive(const char *path1, const char *path2)
+{
+    if ((path1[0] < 'A' || (path1[0] > 'Z' && path1[0] < 'a') || path1[0] > 'Z')
+     || (path2[0] < 'A' || (path2[0] > 'Z' && path2[0] < 'a') || path2[0] > 'Z')
+     || path1[1] != ':' || path2[1] != ':')
+        return 0;

+    /* Once in the domain of ASCII alpha, compare these case insensitive */
+    return ((path1[0] & 0x1f) == (path2[0] & 0x1f));
+}

Re: [Patch] RE: Windows drive letter check fails on lower case current working drive

Posted by "William A. Rowe Jr." <wr...@rowe-clan.net>.
On 1/19/2010 5:19 AM, Bert Huijben wrote:
> 
> The patch was written on the 1.4.x branch but I svn switch'ed it to trunk for easy application.

I would suggest one bit of alternate code that is a bit more condensed, any
concern?;

+static int same_drive(const char *path1, const char *path2)
+{
+    if ((path1[0] < 'A' || (path1[0] > 'Z' && path1[0] < 'a') || path1[0] > 'Z')
+     || (path2[0] < 'A' || (path2[0] > 'Z' && path2[0] < 'a') || path2[0] > 'Z')
+     || path1[1] != ':' || path2[1] != ':')
+        return 0;

+    /* Once in the domain of ASCII alpha, compare these case insensitive */
+    return ((path1[0] & 0x1f) == (path2[0] & 0x1f));
+}

Re: [Patch] RE: Windows drive letter check fails on lower case current working drive

Posted by "William A. Rowe Jr." <wr...@rowe-clan.net>.
On 1/19/2010 5:19 AM, Bert Huijben wrote:
> 	Ping,
> 
> This issue (and mailinglist thread) is now more than two months old. The latest patch adds a testcase showing the issue in the APR testsuite. (There is also an XFail test in the subversion testsuite for this issue)
> 
> The patch was written on the 1.4.x branch but I svn switch'ed it to trunk for easy application.

I'll commit/backport this right after tagging 1.4.2; presuming nobody
notices any issues, this behavioral change will appear in 1.4.3.

I'm still uncomfortable adding behavior-changing patches while we are
attempting to pull off a regression-fix release, as you pointed out with
respect to the hash/table flaw.  So we can let 1.4.3 be a little more
experimental and subject to a little extra scrutiny, while the users
can put their hands already on 1.4.2 and resume using the most recent
apr release with their svn package :)