All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stephen Hemminger <stephen@networkplumber.org>
To: "Toke Høiland-Jørgensen" <toke@toke.dk>
Cc: netdev@vger.kernel.org,
	Kevin Darbyshire-Bryant <ldir@darbyshire-bryant.me.uk>
Subject: Re: [PATCH iproute2 v2] json_print: Fix hidden 64-bit type promotion
Date: Wed, 25 Apr 2018 08:12:10 -0700	[thread overview]
Message-ID: <20180425081210.41846761@xeon-e3> (raw)
In-Reply-To: <87bme75o5r.fsf@toke.dk>

On Wed, 25 Apr 2018 16:57:52 +0200
Toke Høiland-Jørgensen <toke@toke.dk> wrote:

> Stephen Hemminger <stephen@networkplumber.org> writes:
> 
> > On Wed, 25 Apr 2018 16:30:22 +0200
> > Toke Høiland-Jørgensen <toke@toke.dk> wrote:
> >  
> >> print_uint() will silently promote its variable type to uint64_t, but there
> >> is nothing that ensures that the format string specifier passed along with
> >> it fits (and the function name suggest to pass "%u").
> >> 
> >> Fix this by changing print_uint() to use a native 'unsigned int' type, and
> >> introduce a separate print_u64() function for printing 64-bit values. All
> >> call sites that were actually printing 64-bit values using print_uint() are
> >> converted to use print_u64() instead.
> >> 
> >> Since print_int() was already using native int types, just add a
> >> print_s64() to match, but don't convert any call sites.
> >> 
> >> Cc: Kevin Darbyshire-Bryant <ldir@darbyshire-bryant.me.uk>
> >> Signed-off-by: Toke Høiland-Jørgensen <toke@toke.dk>  
> >
> > Yes, this makes sense. Maybe there should be a print_luint for
> > consistency.  
> 
> I just realised I missed a few call sites, so I'll resend. Should I
> call the new function print_luint() instead of print_u64()?

Ideally, there would be both functions, and use based on what is being printed.

> > Also, I tried (in vain) to make a version that allows GCC to check the
> > format string.  But it was a struggle and just gave up.  
> 
> Yeah, no idea how to do this either...

Maybe some magic smatch or multi-line regex it would be possible to
find all instances of print_uint, then look at format string of each and see if there is
a single %u.  Some added complexity since some places only print json and don't care
and pass NULL for format.

  reply	other threads:[~2018-04-25 15:12 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-03-29 19:22 [PATCH iproute2] json_print: fix print_uint hidden type promotion Kevin Darbyshire-Bryant
2018-03-29 21:02 ` Stephen Hemminger
2018-03-29 21:29   ` Kevin Darbyshire-Bryant
2018-04-25 14:30 ` [PATCH iproute2 v2] json_print: Fix hidden 64-bit " Toke Høiland-Jørgensen
2018-04-25 14:55   ` Stephen Hemminger
2018-04-25 14:57     ` Toke Høiland-Jørgensen
2018-04-25 15:12       ` Stephen Hemminger [this message]
2018-04-25 15:28   ` [PATCH iproute2 v3] " Toke Høiland-Jørgensen
2018-04-25 18:11     ` Stephen Hemminger

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=20180425081210.41846761@xeon-e3 \
    --to=stephen@networkplumber.org \
    --cc=ldir@darbyshire-bryant.me.uk \
    --cc=netdev@vger.kernel.org \
    --cc=toke@toke.dk \
    /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.