Lieven Govaerts wrote:
> Stefan Kueng wrote:
> > Julian Foad wrote:
> >> On Wed, 2009-01-07 at 18:18 +0100, Stefan Küng wrote:
> >>> Julian Foad wrote:
> >>>> On Mon, 2008-12-29 at 09:09 -0800, Stefan Kueng wrote:
> >>>>> Author: steveking
> >>>>> Date: Mon Dec 29 09:09:17 2008
> >>>>> New Revision: 34963
> >>>>>
> >>>>> Log:
> >>>>> Fix bug in svn_path_is_canonical() where Windows UNC paths were always
> >>>>> considered non-canonical.
> >>>>>
> >>>>> * subversion/libsvn_subr/dirent_uri.c
> >>>>> (svn_uri_is_canonical): skip the second '/' of an UNC path.
> >>>>>
> >>>>> Modified:
> >>>>> trunk/subversion/libsvn_subr/dirent_uri.c
> >>>>>
> >>>>> Modified: trunk/subversion/libsvn_subr/dirent_uri.c
> >>>>> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_subr/dirent_uri.c?pathrev=34963&r1=34962&r2=34963
> >>>>> ==============================================================================
> >>>>> --- trunk/subversion/libsvn_subr/dirent_uri.c Sun Dec 28 12:03:46 2008 (r34962)
> >>>>> +++ trunk/subversion/libsvn_subr/dirent_uri.c Mon Dec 29 09:09:17 2008 (r34963)
> >>>>> @@ -1140,6 +1140,9 @@ svn_uri_is_canonical(const char *uri)
> >>>>> ! (*(ptr+1) >= 'A' && *(ptr+1) <= 'Z') &&
> >>>>> *(ptr+2) == ':')
> >>>>> return FALSE;
> >>>>> + /* if this is an UNC path, we have two '/' here */
> >>>>> + if (*(ptr+1) == '/')
> >>>>> + ptr++;
> >>>>> }
> >>>>> #endif /* WIN32 or Cygwin */
> >>>> This allows an extra slash in that position for all URIs when running on
> >>>> Windows. Shouldn't this extra check be conditional on "if (strncmp(uri,
> >>>> "file:", 5) == 0"?
> >>> But then plain UNC paths wouldn't work, e.g. //servername/folder/file
> >>>
> >>> Maybe the svn_path_is_canonical() should differ between paths and urls?
> >> This function is svn_uri_is_canonical(), not svn_path_is_canonical().
> >>
> >> Ugh: svn_path_is_canonical() just calls svn_path_is_canonical(). What's
> >> that all about?
> >
>
> I assume you mean svn_uri_is_canonical()?
>
> > That's what I meant with "maybe the svn_path_is_canonical() should
> > differ between paths and urls" :)
> >
> > Haven't checked the latest trunk, but the same applies to
> > svn_path_canonicalize() - in TSVN I had to stop using that API and use
> > svn_uri_canonicalize() and svn_dirent_canonicalize() instead, depending
> > on whether a path or an url is passed.
>
> The svn_path API has always only supported URIs only, so the API has
> been broken on Windows all along. With the introduction of svn_dirent
> and svn_uri we started to fix this, but this work isn't finished. Where
> possible, the svn_path function is now a wrapper around the svn_uri
> function. This should be the same behavior as it was before, as
> demonstrated by the still passing unit tests.
>
> >
> >> There was a TO-DO item talked about before Christmas, something like
> >> "finish or revert the new dirent/URI APIs". Is this an unfinished part
> >> of that?
> >
>
> Why should we revert the API? The only thing left to do is to discuss
> whether the new API is final as is, or if we need to tweak it here and
> there. My concern is mainly in passing a pool to all functions, and
> returning svn_error_t. But there was a suggestion that this can be
> postponed till later, so to update the API where needed in 1.7.
Sorry, I thought I was echoing your thoughts from the email thread
<http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1010183>, but it looks like I misunderstood. I've changed the TODO item to say:
* Review the new "svn_dirent/svn_uri" API and ensure we aren't putting
functions into the public name space that we won't want to support.
See
<http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1010183>.
Is that OK?
- Julian
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1011876
Received on 2009-01-08 16:55:53 CET