On Mon, Nov 08, 2021 at 05:12:16PM +0100, Geert Uytterhoeven wrote:
> Returning void is the other extreme ;-)
>
> There are 3 levels (ignoring BUG_ON()/panic () inside the callee):
> 1. Return void: no one can check success or failure,
> 2. Return an error code: up to the caller to decide,
> 3. Return a __must_check error code: every caller must check.
>
> I'm in favor of 2, as there are several places where it cannot fail.
Makes sense to me. I'll do that in the next iteration.
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
On Mon, Nov 08, 2021 at 05:21:45PM +0100, Borislav Petkov wrote:
> On Mon, Nov 08, 2021 at 05:12:16PM +0100, Geert Uytterhoeven wrote:
> > Returning void is the other extreme ;-)
> >
> > There are 3 levels (ignoring BUG_ON()/panic () inside the callee):
> > 1. Return void: no one can check success or failure,
> > 2. Return an error code: up to the caller to decide,
> > 3. Return a __must_check error code: every caller must check.
> >
> > I'm in favor of 2, as there are several places where it cannot fail.
>
> Makes sense to me. I'll do that in the next iteration.
Is there really any reason for returning an error code? For example, is
it anticipated that at some point in the future these registration calls
might fail?
Currently, the only reason for failing to register a notifier callback
is because the callback is already registered. In a sense this isn't
even an actual failure -- after the registration returns the callback
_will_ still be registered.
So if the call can never really fail, why bother with a return code?
Especially since the caller can't do anything with such a code value.
Given the current state of affairs, I vote in favor of 1 (plus a WARN or
something similar to generate a stack dump in the callee, since double
registration really is a bug).
Alan Stern
On Mon, Nov 08, 2021 at 03:59:26PM -0500, Alan Stern wrote:
> Is there really any reason for returning an error code? For example, is
> it anticipated that at some point in the future these registration calls
> might fail?
>
> Currently, the only reason for failing...
Right, I believe with not making it return void we're leaving the door
open for some, *hypothetical* future return values if we decide we need
to return them too, at some point.
Yes, I can't think of another fact to state besides that the callback
was already registered or return success but who knows what we wanna do
in the future...
And so if we change them all to void now, I think it'll be a lot more
churn to switch back to returning a non-void value and having the
callers who choose to handle that value, do so again.
So, long story short, keeping the retval - albeit not very useful right
now - is probably easier.
I hope I'm making some sense here.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette