On Mon, 2010-11-22, Julian Foad wrote:
> On Mon, 2010-11-22, Bert Huijben wrote:
> > > svn_mergeinfo__add_suffix_to_mergeinfo(svn_mergeinfo_t *out_mergeinfo,
> > > svn_mergeinfo_t mergeinfo,
> > > - const char *suffix,
> > > + const char *suffix_relpath,
> > > apr_pool_t *result_pool,
> > > apr_pool_t *scratch_pool)
> > > {
> [...]
> > > - apr_hash_set(*out_mergeinfo,
> > > - svn_dirent_join(path, canonical_suffix,
> > > result_pool),
> > > - APR_HASH_KEY_STRING,
> > > - svn_rangelist_dup(rangelist, result_pool));
> > > - }
> [...]
> > > + apr_hash_set(*out_mergeinfo,
> > > + svn_dirent_join(path, suffix_relpath, result_pool),
> > > + APR_HASH_KEY_STRING,
> > > + svn_rangelist_dup(rangelist, result_pool));
> >
> > Path is not a dirent (a local disk path), so this should not use the dirent API.
> >
> > I think this should use the new svn_fspath__ join. (Merge info contains a '/')
>
> Yes - that's why I'm looking at this function! (Note: I didn't touch
> that part of it in this commit, just the indentation changed.)
Thinking back, that was a lie. I was looking at the function because it
contained "svn_uri_canonicalize"; I only noticed the two uses of
dirent_* when I went to fix that. And I might not have remembered to
revisit the remaining dirent_join if you hadn't said this, so thank you.
After seeing this, I might go and search for other bad uses of
svn_dirent_* as well, at some point.
- Julian
Received on 2010-11-23 13:21:47 CET