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.
Lieven
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1010215
Received on 2009-01-07 19:30:44 CET