public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Tejun Heo <tj@kernel.org>
To: Joe Perches <joe@perches.com>
Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>,
	Maurizio Lombardi <mlombard@redhat.com>,
	akpm@linux-foundation.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] lib/vsprintf.c: increase the size of the field_width variable
Date: Thu, 10 Sep 2015 10:43:51 -0400	[thread overview]
Message-ID: <20150910144351.GC8114@mtj.duckdns.org> (raw)
In-Reply-To: <1441873056.17219.139.camel@perches.com>

Hello,

On Thu, Sep 10, 2015 at 01:17:36AM -0700, Joe Perches wrote:
> > It also won't work for the case at hand if/when the actual bitmap ever
> > gets a bit set beyond S16_MAX.
> 
> But at least it should work for the bitmap sized <= S16_MAX
> which should be the majority of uses.

The failure mode is too subtle and this is a utility function which is
too fundamental.  I think we really should just get it working
properly.

> > A (somewhat ugly?) solution might be to teach %pb another flag, say h (for
> > huge), meaning that the pointer is actually (struct printf_bitmap*),
> > with 
> > 
> > struct printf_bitmap { unsigned long *bits; unsigned long nbits; }
> > 
> > Then callers with potentially huge bitmaps would do
> > 
> > struct printf_bitmap tmp = { my_bitmap, my_size };
> > snprintf("%pbhl", &tmp)
> 
> Yes, but it still couldn't work without the ability to have
> large output buffers and printk doesn't support that.

printk overrunning its output buffer is fine.  We've always had that
and while we probably should do a better job of indicating those
events, when the output line runs off that long, people (and it's
usually human minds that process printk outputs) already kinda suspect
that.

Long bitmaps don't mean long output and silently formatting the wrong
output sounds like a pretty bad idea and that because we can't make
the width field 32bit?  It doesn't make any sense.

> seq_printf might have some performance issue with it too as
> it would repetitively try to emit, fail, and grow the buffer.

That's kinda beside the point here.

Thanks.

-- 
tejun

  reply	other threads:[~2015-09-10 14:44 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-09-09 10:13 [PATCH] lib/vsprintf.c: increase the size of the field_width variable Maurizio Lombardi
2015-09-09 13:39 ` Tejun Heo
2015-09-09 16:33 ` Joe Perches
2015-09-09 16:36   ` Tejun Heo
2015-09-09 16:55     ` Joe Perches
2015-09-09 18:51   ` Rasmus Villemoes
2015-09-09 19:26     ` Joe Perches
2015-09-10 14:36       ` Tejun Heo
2015-09-10 15:41         ` Joe Perches
2015-09-10 15:47           ` Tejun Heo
2015-09-10  7:04     ` Maurizio Lombardi
2015-09-10  7:13       ` Maurizio Lombardi
2015-09-10  7:38       ` Joe Perches
2015-09-10  7:56         ` Rasmus Villemoes
2015-09-10  8:17           ` Joe Perches
2015-09-10 14:43             ` Tejun Heo [this message]
2015-09-10  8:39         ` Maurizio Lombardi

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=20150910144351.GC8114@mtj.duckdns.org \
    --to=tj@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=joe@perches.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@rasmusvillemoes.dk \
    --cc=mlombard@redhat.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox