Messages in this thread | | | Date | Fri, 30 Sep 2022 15:24:24 +0200 | From | Uwe Kleine-König <> | Subject | Re: [PATCH] printf: Emit "SUCCESS" if NULL is passed for %pe |
| |
On Fri, Sep 30, 2022 at 02:14:16PM +0200, Petr Mladek wrote: > On Fri 2022-09-30 13:10:50, Uwe Kleine-König wrote: > > For code that emits a string representing a usual return value it's > > convenient to have a 0 result in a string representation of success > > instead of "00000000". > > Does it really always mean success, please? > > IMHO, if a function returns a pointer then typically only a valid > pointer means success. Error code means some reasonable explanation > of the failure. And NULL should never happen.
So your example function doesn't hit the case that we're discussing here because it will never return NULL and so the code path I added isn't used and doesn't make a difference, right?
> For example: > > struct bla *find_bla(int key) > { > struct bla *b; > > /* Try to get bla using the given key */ > ... > > if (succeded) > return b; > > /* Did not find bla for the given key */ > return -EINVAL;
nitpick: s/-EINVAL/ERR_PTR(-EINVAL)/
> > } > > It might be used: > > int process_bla() > { > struct bla *b; > > b = get_bla(); > if (IS_ERR(b)) > return PTR_ERR(b); > > /* do something with b */ > ... > } > > If get_bla() returns NULL then it means a super fault. It means > that get_bla() failed and did not know why.
OK, I think we agree that a function that might return an error pointer shouldn't return NULL with the semantic "This is also an error."
Only in combination with such a function you can reasonably object the addition of PTR_ERR(0) meaning "SUCCESS". In such a case the right action is to fix the function.
> IMHO, this patch might do more harm than good.
Hmm, do you think there are many functions that use both NULL and error pointers to signal a failure? I don't see where the patch might do harm otherwise.
In *my* humble opinion it's perfectly fine that a given printk feature results in strange output when it's fed with strange input.
Best regards Uwe
-- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | https://www.pengutronix.de/ | [unhandled content-type:application/pgp-signature] | |