All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Tobin C. Harding" <me@tobin.cc>
To: Adam Borowski <kilobyte@angband.pl>
Cc: Kees Cook <keescook@chromium.org>, Petr Mladek <pmladek@suse.com>,
	Sergey Senozhatsky <sergey.senozhatsky@gmail.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Joe Perches <joe@perches.com>,
	"Roberts, William C" <william.c.roberts@intel.com>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	David Laight <David.Laight@aculab.com>,
	Randy Dunlap <rdunlap@infradead.org>,
	Geert Uytterhoeven <geert@linux-m68k.org>
Subject: Re: [PATCH] vsprintf: avoid misleading "(null)" for %px
Date: Tue, 6 Feb 2018 09:22:54 +1100	[thread overview]
Message-ID: <20180205222254.GS29988@eros> (raw)
In-Reply-To: <20180205205817.72dy7e7xzjcnwmhs@angband.pl>

On Mon, Feb 05, 2018 at 09:58:17PM +0100, Adam Borowski wrote:
> On Tue, Feb 06, 2018 at 07:32:32AM +1100, Kees Cook wrote:
> > On Tue, Feb 6, 2018 at 7:15 AM, Tobin C. Harding <me@tobin.cc> wrote:
> > > On Tue, Feb 06, 2018 at 05:57:17AM +1100, Kees Cook wrote:
> > >> On Mon, Feb 5, 2018 at 8:44 PM, Petr Mladek <pmladek@suse.com> wrote:
> > >> > On Sun 2018-02-04 18:45:21, Adam Borowski wrote:
> > >> >> Like %pK already does, print "00000000" instead.
> > >> >>
> > >> >> This confused people -- the convention is that "(null)" means you tried to
> > >> >> dereference a null pointer as opposed to printing the address.
> > >
> > > Leaving aside what is converting to %px.  If we consider that using %px
> > > is meant to convey to us that we _really_ want the address, in hex hence
> > > the 'x', then it is not surprising that we will get "00000000"'s for a
> > > null pointer, right?  Yes it is different to before but since we are
> > > changing the specifier does this not imply that there may be some
> > > change?
> > 
> > I personally prefer 0000s, but if we're going to change this, we need
> > to be aware of the difference.
> 
> It's easy to paint this bikeshed any color you guys want to: there's an "if"
> already.  My preference is also 0000; NULL would be good, too -- I just
> don't want (null) as that has a special meaning in usual userspace
> implementations; (null) also fits well most other modes of %p as they show
> some object the argument points to.  Confusion = wasted debugging time.
> 
> This is consistent with what we had before, with %pK special-cased.
> 
> > > In what is now to be expected fashion for %p the discussion appears to
> > > have split into two different things - what to do with %px and what to
> > > do with %pK :)
> > 
> > I say leave %pK alone. :)
> 
> As in, printing some random (hashed) value?
> 
> 
> Let's recap:
> 
> Currently:
>               not-null              null
> %pponies      object's description  (null)
> %px           address               (null)
> %pK           hash                  hash
> 
> I'd propose:
>               not-null              null
> %pponies      object's description  (null)
> %px           address               00000000
> %pK           hash                  00000000
> 
> The initial patch in this thread changes printk("%px",0) from (null) to
> 00000000; what Tobin complained about is that printk("%pK",0) prints a
> random value.               

Epic fail on my behalf, my first comment was _wrong_ and brought %pK
into the discussion - bad Tobin, please crawl back under your rock.

The original patch is good IMO and I AFAICT in everyone else's.

	Tobin

  reply	other threads:[~2018-02-05 22:23 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-02-04 17:45 [PATCH] vsprintf: avoid misleading "(null)" for %px Adam Borowski
2018-02-05  9:44 ` Petr Mladek
2018-02-05 10:03   ` Tobin C. Harding
2018-02-05 15:22     ` Adam Borowski
2018-02-05 16:49       ` Steven Rostedt
2018-02-05 17:36         ` Randy Dunlap
2018-02-05 20:19           ` Tobin C. Harding
2018-02-05 18:57   ` Kees Cook
2018-02-05 20:15     ` Tobin C. Harding
2018-02-05 20:32       ` Kees Cook
2018-02-05 20:58         ` Adam Borowski
2018-02-05 22:22           ` Tobin C. Harding [this message]
2018-02-06 18:43             ` Andy Shevchenko
2018-02-07 15:03           ` Petr Mladek
2018-02-07 15:11             ` Geert Uytterhoeven
2018-02-07 15:41               ` Petr Mladek
2018-02-07 15:48                 ` Geert Uytterhoeven
2018-02-08 15:29                 ` Andy Shevchenko
2018-02-09 12:03                   ` Petr Mladek
2018-02-14 14:35                     ` Andy Shevchenko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180205222254.GS29988@eros \
    --to=me@tobin.cc \
    --cc=David.Laight@aculab.com \
    --cc=akpm@linux-foundation.org \
    --cc=geert@linux-m68k.org \
    --cc=joe@perches.com \
    --cc=keescook@chromium.org \
    --cc=kilobyte@angband.pl \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pmladek@suse.com \
    --cc=rdunlap@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=sergey.senozhatsky@gmail.com \
    --cc=torvalds@linux-foundation.org \
    --cc=william.c.roberts@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.