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 11:47:06 -0400 [thread overview]
Message-ID: <20150910154706.GG8114@mtj.duckdns.org> (raw)
In-Reply-To: <1441899685.17219.158.camel@perches.com>
Hello, Joe.
On Thu, Sep 10, 2015 at 08:41:25AM -0700, Joe Perches wrote:
> On Thu, 2015-09-10 at 10:36 -0400, Tejun Heo wrote:
> > On Wed, Sep 09, 2015 at 12:26:39PM -0700, Joe Perches wrote:
> > > > > %*pb is meant for smallish bitmaps, not big ones.
> []
> > The use case isn't from me, but why not?
>
> Imagine the output of the 500k bitmap if every other
> bit is set.
Yeah, but the caller still should be able to call, say, scnprintf()
with a limited buffer and get the output till the end of the buffer
along with the indication that the output has been truncated.
> %*pb isn't capable of multiple line output and
> seq_printf output would also fail as it uses k.alloc
> memory not vmalloc.
Heh, and it should fail.
> I think that a more limited mechanism might be to use a
> multiple line oriented function like print_hex_debug
> and not try to emit the entire thing in a single go.
I'm not that worried about the berserk cases which try to print a
really long output on consoles but the subtle failure modes are
worrying.
> > Why are we even copying the struct on invocations?
> > Only some functions modify the values after all.
> > We might as well pass around pointer to the struct
> > and let the callees wihch modify them copy the
> > fields in local vars like normal functions.
>
> You are of course welcome and able to change it.
>
> btw: the current implementation has a limitation
> on 32 bit arches as it uses an int argument for the
> unsigned long count of bits in a bitmap.
>
> That's a bitmap that should not be printed anyway.
That's 256Mbytes of bitmap. I don't think we need to worry about that
at the moment.
Thanks.
--
tejun
next prev parent reply other threads:[~2015-09-10 15:47 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-09-09 10:13 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 [this message]
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
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=20150910154706.GG8114@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
Powered by JetHome