Re: [PATCH] vsprintf: Do not break early boot with probing addresses

Hi Steve,

On Tue, May 14, 2019 at 8:37 PM Steven Rostedt [off-list ref] wrote:

On Tue, 14 May 2019 11:02:17 +0200
Geert Uytterhoeven [off-list ref] wrote:

quoted

On Tue, May 14, 2019 at 10:29 AM David Laight [off-list ref] wrote:

quoted

quoted

And I like Steven's "(fault)" idea.
How about this:

      if ptr < PAGE_SIZE              -> "(null)"
      if IS_ERR_VALUE(ptr)            -> "(fault)"

      -ss

Or:
        if (ptr < PAGE_SIZE)
                return ptr ? "(null+)" : "(null)";

Hmm, that is useful.

quoted

quoted

        if IS_ERR_VALUE(ptr)
                return "(errno)"

I still prefer "(fault)" as is pretty much all I would expect from a
pointer dereference, even if it is just bad parsing of, say, a parsing
an MAC address. "fault" is generic enough. "errno" will be confusing,
because that's normally a variable not a output.

quoted

Do we care about the value? "(-E%u)"?

That too could be confusing. What would (-E22) be considered by a user
doing an sprintf() on some string. I know that would confuse me, or I
would think that it was what the %pX displayed, and wonder why it
displayed it that way. Whereas "(fault)" is quite obvious for any %p
use case.

I would immediately understand there's a missing IS_ERR() check in a
function that can return  -EINVAL, without having to add a new printk()
to find out what kind of bogus value has been received, and without
having to reboot, and trying to reproduce...

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds