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

Re: [PATCH][merge-tracking] code simplification of get_merge_info_for_path

From: Kamesh Jayachandran <kamesh_at_collab.net>
Date: 2006-10-09 11:51:51 CEST

Malcolm Rowe wrote:
> On Mon, Oct 09, 2006 at 12:00:49PM +0530, Kamesh Jayachandran wrote:
>
>> Use strcpy rather than mimicing the behaviour of 'strcpy'.
>>
>>
>
>
>> @@ -490,8 +488,7 @@
>> path string. */
>> toappend = apr_pcalloc(pool,
>> (strlen(path) - parentpath->len) + 1);
>> - for (i = 0, p = &path[parentpath->len + 1]; *p; i++, p++)
>> - *(toappend + i) = *p;
>> + strcpy(toappend, path+(parentpath->len + 1));
>> append_component_to_paths(&translatedhash, cacheresult,
>> toappend, pool);
>>
>>
>
> Nice find, although I find the &path[n] syntax easier to parse myself
> when n is complex.
>
> However, I don't understand why this function needs to copy part of path
> into pool - do we need to modify toappend later?
>
> i.e. what's wrong with just this?
>
> toappend = &path[parentpath->len + 1];
>
> Alternatively, if you do need to copy it, just call apr_pstrdup() rather
> than pcalloc/strcpy.
>
> Regards,
> Malcolm
>
Yes Malcom, 'toappend' is not modified by
'append_component_to_paths'(Accepts it as const char*).
Attaching the new patch and log.

With regards
Kamesh Jayachandran

[[[
Patch by: Kamesh Jayachandran <kamesh@collab.net>

No need to duplicate the component dir as it is never modified by
function('append_component_to_paths') using it.

* subversion/libsvn_fs_util/merge-info-sqlite-index.c
  (get_merge_info_for_path):
   No need to duplicate the component dir as it is never modified by
   function('append_component_to_paths') using it.
   
]]]

Index: subversion/libsvn_fs_util/merge-info-sqlite-index.c
===================================================================
--- subversion/libsvn_fs_util/merge-info-sqlite-index.c (revision 21840)
+++ subversion/libsvn_fs_util/merge-info-sqlite-index.c (working copy)
@@ -481,17 +481,9 @@
             apr_hash_set(result, path, APR_HASH_KEY_STRING, NULL);
           else if (cacheresult)
             {
- const char *p;
- int i;
               apr_hash_t *translatedhash;
- char *toappend;
+ const char *toappend = &path[parentpath->len + 1];
 
- /* We want to append from the part after the / to the end of the
- path string. */
- toappend = apr_pcalloc(pool,
- (strlen(path) - parentpath->len) + 1);
- for (i = 0, p = &path[parentpath->len + 1]; *p; i++, p++)
- *(toappend + i) = *p;
               append_component_to_paths(&translatedhash, cacheresult,
                                         toappend, pool);
               apr_hash_set(result, path, APR_HASH_KEY_STRING,

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Mon Oct 9 11:51:17 2006

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