[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: rev 2892 - trunk/subversion/mod_dav_svn

From: <brane_at_xbc.nu>
Date: 2002-08-06 18:35:13 CEST

Ben Collins-Sussman wrote:

>Justin Erenkrantz <jerenkrantz@apache.org> writes:
>
>
>
>>On Tue, Aug 06, 2002 at 12:01:17AM -0500, sussman@tigris.org wrote:
>>
>>
>>>@@ -815,6 +815,10 @@
>>> const char *fs_path;
>>> const char *repo_name;
>>> const char *xslt_uri;
>>>+ const char *fs_parent_path;
>>>+ const char *true_root_path;
>>>+ const char *true_fs_path;
>>>+ const char *true_relative_path;
>>>
>>>
>>Just to point out, it probably would have been better if you
>>updated root_path, fs_path, and relative_path in the SVNParentPath
>>case rather than declare new variables. It certainly would have
>>made the patch a lot smaller. You would have contained the
>>scope of your changes to just the else part.
>>
>>
>
>That was my first technique, but then I got paranoid; I wasn't sure if
>it was okay to modify 'root_path' directly, which was coming in as an
>input param to the function. So out of utter paranoia, I abstracted
>the variables...
>
>
>
>
>>>+ else if (fs_parent_path != NULL)
>>>+ {
>>>+ /* SVNParentPath was used: assume the first component of
>>>+ 'relative' is the name of a repository. */
>>>+ const char *magic_component;
>>>+ int i;
>>>+ apr_size_t len = strlen(relative);
>>>+
>>>+ for (i = 1; i < len; i++)
>>>+ {
>>>+ if (relative[i] == '/')
>>>+ break;
>>>+ }
>>>+
>>>+ magic_component = apr_pstrndup(r->pool, relative + 1, i - 1);
>>>+
>>>+ true_relative_path = relative + i;
>>>+ true_root_path = svn_path_join (root_path, magic_component, r->pool);
>>>+ true_fs_path = svn_path_join (fs_parent_path, magic_component, r->pool);
>>>+ }
>>>+
>>>+
>>>
>>>
>>What is the rationale for not using strchr here? That'd be way simpler
>>to deal with, wouldn't it?
>>
>>
>>
>
>I seem to have a memory that "we don't like strchr" for some reason;
>maybe gstein told us not to use it at some point? I notice that we
>almost never use it anywhere in our codebase -- even
>svn_path_decompose() is using the same technique above. I'll switch
>to strchr if people prefer that.
>

I can't for the life of me understand why we'd not use strchr. Apart
from being faster in general (as it's at least a call to a
hand-optimized libarary function, and often the compiler will expand
optimal code inline), it's also moch more obvious what's happening. So,
+1 for replacing all such loops with the standard string functions.

-- 
Brane Čibej   <brane_at_xbc.nu>   http://www.xbc.nu/brane/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Aug 6 18:35:58 2002

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.