Re: [PATCH 11/29] bisect: libify `bisect_next_all`

On Mon, Jan 20, 2020 at 11:29 PM Johannes Schindelin
[off-list ref] wrote:

On Mon, 20 Jan 2020, Miriam Rubio wrote:

quoted

Since we want to get rid of git-bisect.sh it would be necessary to
convert those exit() calls to return statements so that errors can be
reported.

Emulate try catch in C by converting `exit(<positive-value>)` to
`return <negative-value>`. Follow POSIX conventions to return
<negative-value> to indicate error.

Turn `exit()` to `return` calls in `bisect_next_all()`.

All the functions calling `bisect_next_all()` are already able to
handle return values from it.

So this patch brings more magic values that really would like to become
`enum` values. At this point, we're talking about `bisect_next_all()`
which is _not_ a file-local (`static`) function. Therefore, we now really
wade into API territory where an `enum` is no longer just a nice thing,
but a necessary thing: `bisect_next_all()` is a function that can be
called from other source files.

I agree that return values could be better documented. About enum
though, I am perhaps wrong but I don't think that we use them often
for error codes. Do you have examples in the code base where they are
used for such a purpose along with regular `error(...)` calls?

As far as I can see, this patch concludes the "libify" part of the patch
series (read: it would have been the second patch series I would have
suggested to split out from an otherwise too-long-to-be-reviewed set of
patches, if I had been your mentor),

I am ok with saying that "part 1" stops after this patch and that the
rest will be "part 2" and will not be resent in the version 2 of the
"part 1" patch series, but will rather be resent later, when "part 1"
will be in next for example.

and I'll leave it at that for
tonight; Hopefully I will find some time to review more of these patches
tomorrow.

That would be great!

Thanks,
Christian.