Messages in this thread |  | | | Date | Wed, 9 May 2012 16:15:28 +0200 | | From | Ingo Molnar <> | | Subject | Re: [RFC][PATCH] printk: Add %pb to print bitmaps |
| |
* Peter Zijlstra <peterz@infradead.org> wrote:
> On Wed, 2012-05-09 at 15:44 +0200, Peter Zijlstra wrote: > > On Wed, 2012-05-09 at 15:36 +0200, Ingo Molnar wrote: > > > * Peter Zijlstra <peterz@infradead.org> wrote: > > > > > > > @@ -857,15 +860,19 @@ int kptr_restrict __read_mostly; > > > > * correctness of the format string and va_list arguments. > > > > * - 'K' For a kernel pointer that should be hidden from unprivileged users > > > > * - 'NF' For a netdev_features_t > > > > + * - 'b' For a bitmap, consumes 2 args, second is int > > > > > > hm, won't the second arg confuse gcc's printf format checker? > > > > > > Ah, yes, I suppose I could abuse something like %*pb. Let me try that. > > I guess I should use %.*pb and keep the field_width in case someone > manages to actually make bitmap_scnlistprintf() conform to it. The > precision is unused anyway.
That's a cute trick, and it's intuitive as well.
> + case 'b': > + { > + int bits, len; > + > + switch (fmt[1]) { > + case 'c': > + bits = nr_cpumask_bits; > + break; > + case 'n': > + bits = MAX_NUMNODES; > + break; > + default: > + bits = spec->precision; > + break;
So, if someone specifies an incomplete "%pb" format - fmt[1] will be 0 and we take precision as the length - presumably also 0. We stick that 0 into:
> + len = bitmap_scnlistprintf(buf, end - buf, ptr, bits);
Will that work?
Provided it all tests out fine for you it looks good to me.
I'd probably write the switch statement as:
switch (fmt[1]) { case 'c': bits = nr_cpumask_bits; break; case 'n': bits = MAX_NUMNODES; break; default: bits = spec->precision; break; } ... but that's a small detail and a matter of taste in any case.
Thanks,
Ingo
|  |