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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Chris,

On Tue, 21 Jan 2020, Christian Couder wrote:

> On Mon, Jan 20, 2020 at 11:29 PM Johannes Schindelin
> <Johannes.Schindelin@xxxxxx> wrote:
>
> > On Mon, 20 Jan 2020, Miriam Rubio wrote:
>
> > > 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.

It does not matter how often we actually use them, it matters more whether
we _want_ to use them. And in the recent years, we have definitely made
more use of enums than before, to allow the compiler to catch mistakes
earlier. We even started to convert existing constants to enums, for
example.

So I don't think that it is sound advice here to point to the code base.

If you _must_ value the existing practice over a clearly communicated
review, then please look no further than at the declaration of
`safe_create_leading_directories()`.

> Do you have examples in the code base where they are used for such a
> purpose along with regular `error(...)` calls?

It's all in the code. You don't need me to read it aloud for you.

:-)

Ciao,
Dscho




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux