Hi Dirk,

On Tue, 27 Feb 2024, Dirk Gouders wrote:

"Johannes Schindelin via GitGitGadget" [off-list ref] writes:

quoted

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:

quoted

-		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

quoted

+			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