Karl Fogel wrote:
> I think svn_opt__split_arg_at_peg_revision() is buggy. This discovery
> was spurred by a bug report (quoted at the end of this mail).
>
> Here's the svn_opt__split_arg_at_peg_revision API:
>
> /* Extract the peg revision, if any, from UTF8_TARGET. Return the peg
> * revision in *PEG_REVISION and the true target portion in *TRUE_TARGET.
> * *PEG_REVISION will be an empty string if no peg revision is found.
> *
> * UTF8_TARGET need not be canonical. *TRUE_TARGET will not be canonical
> * unless UTF8_TARGET is.
> *
> * Note that *PEG_REVISION will still contain the '@' symbol as the first
> * character if a peg revision was found.
> *
> * All allocations are done in POOL.
> */
> svn_error_t *
> svn_opt__split_arg_at_peg_revision(const char **true_target,
> const char **peg_revision,
> const char *utf8_target,
> apr_pool_t *pool);
[...]
> SVN_ERR(svn_opt__split_arg_at_peg_revision(&true_target, &peg_rev,
> utf8_target, pool));
[...]
> ### Hmmm, I want to test what happens with "dir/@file@" instead,
> ### so I'll just set utf8_target to that:
[...]
> (gdb) p true_target
> $11 = 0x9e47598 "dir/@file"
> (gdb) p peg_rev
> $12 = 0x9e475a8 "@"
>
> ### Mmmm. Yup. So, 'peg_rev' should really be "", right?
No. It is by design that this function returns the entire peg-specifier
part of the input string, rather than a canonical peg specifier derived
from it. The callers rely on this.
Now, it's maybe not a great design, but that's how it is at the moment.
And the doc string is not clear on this point.
- Julian
> Should I just make the obvious fix -- that is, set 'peg_start' to NULL
> if we're on the first iteration of the loop -- or is there something
> deeper going on here?
[...]
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-08-27 22:45:07 CEST