Stefan Fuhrmann wrote on Sun, 21 May 2017 20:09 +0200:
> On 15.11.2016 00:43, Daniel Shahaf wrote:
> > Stefan Fuhrmann wrote on Mon, Nov 14, 2016 at 12:50:51 +0100:
> >> On 06.11.2016 02:21, Daniel Shahaf wrote:
> >>> During the r1767197 thread, I noticed that vwrite_tuple() doesn't
> >>> enforce its precondition that in the optional part of a tuple, either
> >>> all values are valid or no value is; a call such as
> >>>
> >>> vwrite_tuple("(?r?r)", SVN_INVALID_REVNUM, (svn_revnum_t) 42)
> >>>
> >>> would happily generate a «( 42 ) » tuple, instead of triggering an
> >>> SVN_ERR_MALFUNCTION().
> >>>
> >>> This would be easy to fix, and would prevent a class of bugs.
> >> This function seems to have a number of bugs,
> >> for instance:
> >>
> >> * optional sub-structs don't get a "("
> >> * the OPT flag is global instead of per-struct
> >
> > I hope at least the parser routines don't have such bugs.
> >
> > (excuse brevity; can't be verbose at the moment)
>
> As it turns out, the current implementation is correct but
> forbids optional sub-tuples. See r1795714.
r1795714 broke buildbot. I think that's because it caused the '(' case
to advance ap, whereas before that revision it didn't. (Thta change
wasn't mentioned in the log message, by the way.)
Thanks for getting to the bottom of this. I assume the other cases discussed
in this thread are also fine...
Cheers,
Daniel
Received on 2017-05-21 20:28:10 CEST