Hi Dirk,
On Tue, 27 Feb 2024, Dirk Gouders wrote:
> "Johannes Schindelin via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
>
> > Let's start at the bottom of the stack by teaching the
> > `paint_down_to_common()` function to return an `int`: if negative, it
> > indicates fatal error, if 0 success.
>
> Kind of pedantic but the above doesn't describe the real change, i.e. a
> value != 0 indicates a fatal error:
>
> > - common = paint_down_to_common(r, array[i], filled,
> > - work, min_generation, 0);
> > + if (paint_down_to_common(r, array[i], filled,
> > + work, min_generation, 0, &common)) {
The fact that we do not bother to verify that the return value is
negative, but only check for a non-zero one instead, does not change the
fact that in the form this patch leaves the code, `paint_down_to_common()`
returns -1 for fatal errors and 0 for success, as advertised, though.
Is it lazy to omit the `< 0` here? Not actually, the reason why I omitted
it here was to stay under 80 columns per line.
Good eyes, though. If `paint_down_to_common()` _did_ return values other
than -1 and 0, in particular positive ones that would not indicate a fatal
error, the hunk under discussion would have introduced a problematic bug.
Ciao,
Johannes
> > + clear_commit_marks(array[i], all_flags);
> > + clear_commit_marks_many(filled, work, all_flags);
> > + free_commit_list(common);
> > + free(work);
> > + free(redundant);
> > + free(filled_index);
> > + return -1;
> > + }
>
> Dirk
>
>
