All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Laight <David.Laight@ACULAB.COM>
To: 'Stephen Hemminger' <stephen@networkplumber.org>,
	Hangbin Liu <liuhangbin@gmail.com>
Cc: "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"jhs@mojatatu.com" <jhs@mojatatu.com>
Subject: RE: [PATCH iproute2] tc: u32: check return value from snprintf
Date: Fri, 16 Feb 2024 09:58:21 +0000	[thread overview]
Message-ID: <28c9af6b1ddf4664bbfed973e2f01874@AcuMS.aculab.com> (raw)
In-Reply-To: <20240212085358.22be1db6@hermes.local>

From: Stephen Hemminger <stephen@networkplumber.org>
> Sent: 12 February 2024 16:54
> 
> On Mon, 12 Feb 2024 15:24:49 +0800
> Hangbin Liu <liuhangbin@gmail.com> wrote:
> 
> > On Sat, Feb 10, 2024 at 05:04:23PM -0800, Stephen Hemminger wrote:
> > > Add assertion to check for case of snprintf failing (bad format?)
> > > or buffer getting full.
> > >
> > > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> >
> > Hi Stephen,
> >
> > Is there a bug report or something else that we only do the assertion
> > for tc/f_u32.c?
> >
> > Thanks
> > Hangbin
> 
> No bug, it is not possible to trigger with current code.
> Return of < 0 only happens with improper format string,
> and the overrun would only happen if buffer was not big enough
> The bsize is SPRINT_BUF() which is 64 bytes.
> 
> It is more a way to avoid some code checker complaining in future.

What are you testing?
If you are testing snprintf() then maybe a check for <0 is sane.
But otherwise it is a waste of time.

FWIW do you know what (any) printf() function should return for (eg):
	int len = MAXINT;
	len = snprintf(NULL, 0, "%*s %*s", len, "abcd", len, "1234");

My brain doesn't think that a 'bad format' generates -1.
You can get -1 from fprintf() (if a write() fails) which largely
means that code looking at the result of fprintf() is broken.
(You need to do fflush() and ferror() if you want to know
if a write failed, and you can't reliably assume it is the
length.)

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)


  reply	other threads:[~2024-02-16  9:58 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-11  1:04 [PATCH iproute2] tc: u32: check return value from snprintf Stephen Hemminger
2024-02-12  7:24 ` Hangbin Liu
2024-02-12 16:53   ` Stephen Hemminger
2024-02-16  9:58     ` David Laight [this message]
2024-02-13  6:25 ` Hangbin Liu
2024-02-15  3:10 ` patchwork-bot+netdevbpf

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=28c9af6b1ddf4664bbfed973e2f01874@AcuMS.aculab.com \
    --to=david.laight@aculab.com \
    --cc=jhs@mojatatu.com \
    --cc=liuhangbin@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=stephen@networkplumber.org \
    /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.