Re: [PATCH v2] reftable/writer: ensure valid range for log's update_index

[Junio C Hamano <gitster@pobox.com>]

Karthik Nayak <karthik.188@gmail.com> writes:

> The corresponding check for reflogs in `reftable_writer_add_log` is
> however missing. Add a similar check, but only check for the upper
> limit. This is because reflogs are treated a bit differently than refs.
> Each reflog entry in reftable has an associated update_index and we also
> allow expiring entries in the middle, which is done by simply writing a
> new reflog entry with the same update_index. This means, writing reflog
> entries with update_index lesser than the writer's update_index is an
> expected scenario.
>
> Add a new unit test to check for the limits and fix some of the existing
> tests, which were setting arbitrary values for the update_index by
> ensuring they stay within the now checked limits.

Interesting.  I am a little curious how this was found.  As long as
the other parts of the system (i.e. the callers) are not buggy, the
update pointer will stay within the range, and that is why I do not
think I can write an end-to-end test to trigger an existing bug that
would be caught by this "fix".  Fixing the existing unit tests that
feed a wrong input and expect some right outcome is good, but would
it be a good to also have a new unit test that validates that such
an incorrect precondition for calling is caught and the caller gets
the expected REFTABLE_API_ERROR as a result, I wonder?  Being able
to trigger a condition that is harder to do with the end-to-end test
is one good thing about having unit test framework, no?

Will queue.  Thanks.