Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1221985

Re: [PATCH] lib/vsprintf.c: increase the size of the field_width variable

Show all headers | View raw

On Thu, Sep 10 2015, Joe Perches <joe@perches.com> wrote:

> On Thu, 2015-09-10 at 09:04 +0200, Maurizio Lombardi wrote:
>> Hi Rasmus,
>> 
>> On 09/09/2015 08:51 PM, Rasmus Villemoes wrote:
>> > I'm also a little confused; I don't see what printk has to do with the
>> > reported problem (I'd expect the /sys/... file to be generated by
>> > something like seq_printf).
>> 
>> In the scsi-debug case scnprintf is used, but it doesn't really matter
>> because the change I made would influence printk and all its friends as
>> well... everything that will parse "%*pb[l]".
>> 
>> > 
>> >> %*pb is meant for smallish bitmaps, not big ones.
>> > 
>> > Yup. And that leads to my other confusion: Given that the expected
>> > output is given as "0-15", does the bitmap really consist of > S16_MAX
>> > bits with only the first 16 set?
>> > 
>> 
>> Yes. To be precise, in the example I mentioned in the commit message, a
>> bitmap of size = 524288 bits is created.
>> If you assign this number to a s16 variable the result will be zero and
>> nothing will be printed.
>
> Maurizio, did you try the patch I posted?
> I think it'll work, but it doesn't fix the
> fundamental issue of %*pbl with large bitmaps.

It also won't work for the case at hand if/when the actual bitmap ever
gets a bit set beyond S16_MAX.

>> scsi-debug used the bitmap_scnlistprintf() function but since commit
>> dbc760bcc150cc27160f0131b15db76350df4334 this function is just a wrapper
>> around scnprintf("%*pbl");
>> as a consequence, the scsi-debug map_show() function stopped working
>> correctly.
>
> Perhaps the thin wrapper conversions in lib/bitmap.c
> in that commit for bitmap_scnprintf, bscnl_emit, and 
> bitmap_scnlistprintf should be reverted.

Yes, that's the obvious short-term solution, though I think it's a bit
sad having to have library functions needed only by very few users
(after all, it took 6 months for this to be reported).

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)

and I think we'd only need a tiny refactorization in lib/vsprintf.c to
support this. (When using this form, the field width would just be
ignored.) Basically, move the simplistic flag handling from the generic
dispatch function pointer() to its own little helper, let that helper
compute/fetch the (unsigned long*) and size we need, and pass that to
either bitmap_list_string or bitmap_string - basically, they'd lose the
spec argument and grow an explicit nbits argument, and then get to
declare their own struct printf_spec.

Rasmus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH] lib/vsprintf.c: increase the size of the field_width variable Maurizio Lombardi <mlombard@redhat.com> - 2015-09-09 12:20 +0200
  Re: [PATCH] lib/vsprintf.c: increase the size of the field_width  variable Tejun Heo <tj@kernel.org> - 2015-09-09 15:40 +0200
  Re: [PATCH] lib/vsprintf.c: increase the size of the field_width  variable Joe Perches <joe@perches.com> - 2015-09-09 18:40 +0200
    Re: [PATCH] lib/vsprintf.c: increase the size of the field_width  variable Tejun Heo <tj@kernel.org> - 2015-09-09 18:40 +0200
      Re: [PATCH] lib/vsprintf.c: increase the size of the field_width  variable Joe Perches <joe@perches.com> - 2015-09-09 19:00 +0200
    Re: [PATCH] lib/vsprintf.c: increase the size of the field_width variable Rasmus Villemoes <linux@rasmusvillemoes.dk> - 2015-09-09 21:00 +0200
      Re: [PATCH] lib/vsprintf.c: increase the size of the field_width  variable Joe Perches <joe@perches.com> - 2015-09-09 21:30 +0200
        Re: [PATCH] lib/vsprintf.c: increase the size of the field_width  variable Tejun Heo <tj@kernel.org> - 2015-09-10 16:40 +0200
          Re: [PATCH] lib/vsprintf.c: increase the size of the field_width  variable Joe Perches <joe@perches.com> - 2015-09-10 17:50 +0200
            Re: [PATCH] lib/vsprintf.c: increase the size of the field_width  variable Tejun Heo <tj@kernel.org> - 2015-09-10 17:50 +0200
      Re: [PATCH] lib/vsprintf.c: increase the size of the field_width  variable Maurizio Lombardi <mlombard@redhat.com> - 2015-09-10 09:10 +0200
        Re: [PATCH] lib/vsprintf.c: increase the size of the field_width  variable Maurizio Lombardi <mlombard@redhat.com> - 2015-09-10 09:20 +0200
        Re: [PATCH] lib/vsprintf.c: increase the size of the field_width  variable Joe Perches <joe@perches.com> - 2015-09-10 09:40 +0200
          Re: [PATCH] lib/vsprintf.c: increase the size of the field_width variable Rasmus Villemoes <linux@rasmusvillemoes.dk> - 2015-09-10 10:00 +0200
            Re: [PATCH] lib/vsprintf.c: increase the size of the field_width  variable Joe Perches <joe@perches.com> - 2015-09-10 10:20 +0200
              Re: [PATCH] lib/vsprintf.c: increase the size of the field_width  variable Tejun Heo <tj@kernel.org> - 2015-09-10 16:50 +0200
          Re: [PATCH] lib/vsprintf.c: increase the size of the field_width  variable Maurizio Lombardi <mlombard@redhat.com> - 2015-09-10 10:40 +0200

csiph-web