Now I have made some progress. The relevant commits are noted in the
issue tracker.
> Ill-defined canonical form for a rangelist.
Not yet tackled. It doesn't seem to be an immediate blocker.
> Rangelist "merge" is a set-union operation.
Not re-written yet. This is one of the next steps; see below.
> The function range_to_string(), bogus output of empty range
Fixed.
> Poor and inconsistent error handling in svn_sort__array_insert and
_delete.
Fixed. I changed all the "abort" code paths I could find in the area
(libsvn_subr/mergeinfo.c), to return catchable assertion errors.
> Our testing is clearly inadequate.
I have added thorough random-input testing for svn_rangelist_merge2().
It shows the bug of non-canonical output from canonical inputs (see below).
I investigated the next level up the call stack: svn_mergeinfo_merge2(),
and added a random-input crash test for it. It is a simple wrapper
around svn_rangelist_merge2() and given valid inputs there doesn't seem
to be any way it could invoke assertion failures (except for bugs in its
callees such as the "non-canonical output" bug just mentioned).
* Other mergeinfo arithmetic functions still need random-input testing.
=== Next steps ===
* Investigate further up the call stack in a similar way:
combine_forked_mergeinfo_props()
* Re-write svn_rangelist_merge2() as a neat little understandable
algorithm, thereby fixing the revealed bug (non-canonical output from
canonical inputs), now that it has decent regression tests.
(Does anybody else fancy taking on that last one? Sounds like a
potentially fun little algorithm rewrite task.)
- Julian
