Paul Burba wrote on Wed, 13 May 2009 at 10:22 -0400:
> On Wed, May 13, 2009 at 1:37 AM, Daniel Shahaf <d.s_at_daniel.shahaf.name> wrote:
> >> > Paul Burba wrote on Tue, 12 May 2009 at 13:49 -0400:
> >> >> * Unlike mergeinfo with '\r', other versioned and unversioned props
> >> >> are loaded intact, that is to say the '\r\n' are still present in the
> >> >> loaded repos. My patch "fixes" the loaded mergeinfo props so there
> >> >> are no '\r'. I could make loaded mergeinfo be treated the same way,
> >> >> but I see no point in doing this!
....
> >
> > You've convinced me. :-)
> >
> > For the record, I thought we could just pass through the value
> > unmodified (at least when we aren't doing any renumbering at all);
> > however, it appears the logic doesn't take that shortcut explicitly,
> > so that's not possible.
>
> Daniel,
>
> We could tweak load.c:renumber_mergeinfo_revs() to simply set
> *FINAL_VAL to INITIAL_VAL if no renumbering is done, but that is a
> separate issue yes?
Yes, I suppose we could rewrite the logic to detect the "no renumbering"
case more explicitly if we wanted to. Yes, I don't think there is
a reason to do that right now (regardless of any CRLF fixes we have in
mind).
> Regardless of whether or not that change is made,
> load.c:renumber_mergeinfo_revs() needs to call svn_mergeinfo_parse()
> *before* it can determine if renumbering is necessary. So if we don't
> clean up the windows EOLs before then the load will fail when
> svn_mergeinfo_parse() encounters those.
>
> > I see an strstr(svn_string_t.data) --- which normally makes me ask what
> > happens when the svn_string_t's value contains NULs --- but I suppose
> > the answer (in this case) is that svn:mergeinfo cannot legitimately
> > contain NULs.
>
> read_key_or_val(), which provides both keybuf and propstring (the
> arguments to strcmp and strstr respectively), always returns at least
> an empty string, so this is safe. The svn_repos_parse_fns2_t
> set_node_property() immediately following makes the same assumption.
>
So, the terminating NUL will be there, and embedded NULs cannot happen
for svn:mergeinfo. Thanks for clarifying.
> Paul
>
Daniel
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2246035
Received on 2009-05-14 01:41:09 CEST