Lieven Govaerts wrote:
> what's the difference between upper and lower case drive letters on
> Windows? I thought Windows' use of drive letters was case insensitve?
>
> On Wed, Sep 30, 2009 at 10:08 AM, Bert Huijben <rhuijben_at_sharpsvn.net> wrote:
> > Author: rhuijben
> > Date: Wed Sep 30 01:08:37 2009
> > New Revision: 39690
> >
> > Log:
> > Make svn_dirent_is_absolute() only return true when
> > svn_dirent_get_absolute() called on that same path returns the same value.
But svn_dirent_get_absolute() doesn't specify what it does with a
lower-case drive letter. Sure, you know what one particular
implementation of it does.
> > This resolves an issue with lower cased drive letters in Windows, that
> > where found.
> >
> > Found by: rdonch
> >
> > * subversion/libsvn_subr/dirent_uri.c
> > (svn_dirent_is_absolute): Don't assume a lowercase drive letter is absolute.
> >
> > * subversion/tests/libsvn_subr/dirent_uri-test.c
> > (test_dirent_is_absolute): Add two test values, verifying the new behavior and
> > add verification on svn_dirent_get_absolute() value being the same for absolute
> > paths.
> > (test_dirent_get_absolute): Add three test values.
> >
> > Modified:
> > trunk/subversion/libsvn_subr/dirent_uri.c
> > trunk/subversion/tests/libsvn_subr/dirent_uri-test.c
> >
> > Modified: trunk/subversion/libsvn_subr/dirent_uri.c
> > URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_subr/dirent_uri.c?pathrev=39690&r1=39689&r2=39690
> > ==============================================================================
> > --- trunk/subversion/libsvn_subr/dirent_uri.c Wed Sep 30 00:21:55 2009 (r39689)
> > +++ trunk/subversion/libsvn_subr/dirent_uri.c Wed Sep 30 01:08:37 2009 (r39690)
> > @@ -1499,8 +1499,7 @@ svn_dirent_is_absolute(const char *diren
> > /* On Windows, dirent is also absolute when it starts with 'H:' or 'H:/'
> > where 'H' is any letter. */
That comment doesn't match the code: the code requires the "/". (I know
you didn't change this in this patch.)
> > #if defined(WIN32) || defined(__CYGWIN__)
> > - if (((dirent[0] >= 'A' && dirent[0] <= 'Z') ||
> > - (dirent[0] >= 'a' && dirent[0] <= 'z')) &&
> > + if (((dirent[0] >= 'A' && dirent[0] <= 'Z')) &&
> > (dirent[1] == ':') && (dirent[2] == '/'))
> > return TRUE;
> > #endif /* WIN32 or Cygwin */
> >
> > Modified: trunk/subversion/tests/libsvn_subr/dirent_uri-test.c
> > URL: http://svn.collab.net/viewvc/svn/trunk/subversion/tests/libsvn_subr/dirent_uri-test.c?pathrev=39690&r1=39689&r2=39690
> > ==============================================================================
> > --- trunk/subversion/tests/libsvn_subr/dirent_uri-test.c Wed Sep 30 00:21:55 2009 (r39689)
> > +++ trunk/subversion/tests/libsvn_subr/dirent_uri-test.c Wed Sep 30 01:08:37 2009 (r39690)
> > @@ -151,6 +151,8 @@ test_dirent_is_absolute(apr_pool_t *pool
> > { "/", FALSE },
> > { "X:/foo", TRUE },
> > { "X:/", TRUE },
> > + { "x:/", FALSE },
> > + { "x:/foo", FALSE },
>
> It just seems wrong to me to handle dirents differently on Windows
> when only their case differs.
It would be OK to make dirent_is_absolute() be undefined or abort on
non-canonical input, but it is wrong to test that it returns FALSE as
that is not part of the promised behaviour, just an implementation
detail.
> > { "//srv/shr", TRUE },
> > { "//srv/shr/fld", TRUE },
> > { "//srv/s r", TRUE },
> > @@ -178,6 +180,25 @@ test_dirent_is_absolute(apr_pool_t *pool
> > "svn_dirent_is_absolute (%s) returned %s instead of %s",
> > tests[i].path, retval ? "TRUE" : "FALSE",
> > tests[i].result ? "TRUE" : "FALSE");
> > +
+ /* Check that svn_dirent_get_absolute() returns the path
+ unchanged if it's absolute or changed if it's relative. */
> > + /* Don't get absolute paths for the UNC paths, because this will
> > + always fail */
Can't svn_dirent_get_absolute() just return a UNC path unchanged?
> > + if (tests[i].result &&
> > + strncmp(tests[i].path, "//", 2) != 0)
> > + {
> > + const char *abspath;
> > +
> > + SVN_ERR(svn_dirent_get_absolute(&abspath, tests[i].path, pool));
> > +
> > + if (tests[i].result != (strcmp(tests[i].path, abspath) == 0))
> > + return svn_error_createf(
> > + SVN_ERR_TEST_FAILED,
> > + NULL,
> > + "svn_dirent_is_absolute(%s) returned TRUE, but "
> > + "svn_dirent_get_absolute() returned \"%s\"",
> > + tests[i].path,
> > + abspath);
> > + }
>
> This part doesn't seem really related to the rest of the patch.
>
> Lieven
- Julian
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2402027
Received on 2009-09-30 13:55:07 CEST