Re: [PATCH v2 09/11] bisect: libify `check_good_are_ancestors_of_bad` and its dependents

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

[-- Attachment #1: Type: text/plain, Size: 1496 bytes --]

Hi Miriam,

On Thu, 30 Jan 2020, Miriam R. wrote:

> El jue., 30 ene. 2020 a las 14:46, Johannes Schindelin
> (<Johannes.Schindelin@gmx.de>) escribió:
> >
> > On Tue, 28 Jan 2020, Miriam Rubio wrote:
> >
> > > +
> > > +     return res < 0 ? -res : res;
> >
> > This is a change in behavior, though: previously we guaranteed that the
> > exit code is either 0 on success or 1 upon failure. I am not quite sure
> > that we want to change that behavior.
>
> I think this is because different error codes might enable a bisecting
> script calling the bisect command that uses this function to do
> different things depending on the exit status of the bisect command.

Oops. I am _totally_ wrong on this.

As you are changing a lot of `exit(<n>)` to `return -<n>` with the
intention to turn the value into an exit code only at the
`cmd_bisect__helper()` level, this is actually required a change.

I am a bit uneasy about this, but I could not see any return values in
`bisect.c` other than 0 and -1, prior to this patch series.

However, I would love to see this refactored into its own commit, more
toward the beginning of the patch series, with a very clean commit message
that describes that intention of being _the_ exit point from `bisect.c`.

Without this change, you simply cannot change the `exit(<n>);` calls in
`bisect.c` for any `<n>` other than 0 or 1.

Sorry that it took me so long to wrap my head around this rather trivial
idea.

Ciao,
Dscho