linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Maurizio Lombardi <mlombard@redhat.com>
To: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Cc: tj@kernel.org, joe@perches.com, linux-kernel@vger.kernel.org
Subject: Re: [RFC PATCH 0/3] fix *pbl format support
Date: Wed, 16 Sep 2015 14:53:56 +0200	[thread overview]
Message-ID: <55F96664.8040701@redhat.com> (raw)
In-Reply-To: <87d1xi4kic.fsf@rasmusvillemoes.dk>

Hi,

On 09/16/2015 02:27 PM, Rasmus Villemoes wrote:
> 
> If we want to fix the problem with 3/3, then this seems obviously
> necessary. There may be stuff we want to optimize later (for example, I
> don't think we should always make a local copy of the entire struct;

Yes I know, I just tried not to break anything in the process,
optimizations can be done later.

> I haven't looked carefully at your code, but it does seem that you make
> sure that at least the return value is as expected, which will make
> kasprintf work. But it seems there is another kasprintf
> problem. [reminder: kasprintf works by doing a va_copy, then doing a
> first call of vsnprintf, passing NULL for the buffer and 0 for the
> length to determine the size to allocate, and then doing the actual
> formatting with a second call]

Ah, you're right, PATCH 2 is broken because I didn't think to the case
you described.
Please ignore it, thanks for catching this.

> I'm not yet completely convinced this is the right solution. Obviously,
> if other problems with the small .field_width size show up, this might
> be necessary, but as long as it's only the %pb formatter (and so far
> only a single user of that), I think smaller/other hammers should be
> thought about. So far I think there've been two alternatives: (1)
> reintroduce the dedicated bitmap pretty printer(s)

I have no problem with that, at least it will work again.

Thanks for the review,
Maurizio Lombardi

  reply	other threads:[~2015-09-16 12:53 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-09-16  9:08 [RFC PATCH 0/3] fix *pbl format support Maurizio Lombardi
2015-09-16  9:08 ` [RFC PATCH 1/3] lib/vsprintf.c: Do not pass printf_spec by value on stack Maurizio Lombardi
2015-09-16  9:08 ` [RFC PATCH 2/3] lib/vsprintf.c: append "..." if the *pb[l] output has been truncated Maurizio Lombardi
2015-09-16  9:08 ` [RFC PATCH 3/3] lib/vsprintf.c: increase the size of the field_width variable Maurizio Lombardi
2015-09-16 12:27 ` [RFC PATCH 0/3] fix *pbl format support Rasmus Villemoes
2015-09-16 12:53   ` Maurizio Lombardi [this message]
2015-09-16 17:45   ` Tejun Heo
2015-09-16 20:35     ` Rasmus Villemoes
2015-09-21 14:54       ` Maurizio Lombardi
2015-09-21 16:24         ` Rasmus Villemoes

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=55F96664.8040701@redhat.com \
    --to=mlombard@redhat.com \
    --cc=joe@perches.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@rasmusvillemoes.dk \
    --cc=tj@kernel.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).