Re: [PATCH] printf: Emit "SUCCESS" if NULL is passed for %pe

[Uwe Kleine-König]

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/ |

Attachment: signature.asc
Description: PGP signature