Messages in this thread Patch in this message | | | Date | Thu, 3 Dec 2015 15:34:07 -0800 | From | Andrew Morton <> | Subject | Re: [PATCH v3 04/14] lib/vsprintf.c: expand field_width to 24 bits |
| |
On Thu, 03 Dec 2015 23:28:58 +0200 Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> On Thu, 2015-12-03 at 12:54 -0800, Joe Perches wrote: > > On Thu, 2015-12-03 at 21:51 +0100, Rasmus Villemoes wrote: > > > Maurizio Lombardi reported a problem [1] with the %pb extension: It > > > doesn't work for sufficiently large bitmaps, since the size is > > > stashed > > > in the field_width field of the struct printf_spec, which is > > > currently > > > an s16. Concretely, this manifested itself in > > > /sys/bus/pseudo/drivers/scsi_debug/map being empty, since the > > > bitmap > > > printer got a size of 0, which is the 16 bit truncation of the > > > actual > > > bitmap size. > > > > > > We do want to keep struct printf_spec at 8 bytes so that it can > > > cheaply be passed by value. The qualifier field is only used for > > > internal bookkeeping in format_decode, so we might as well use a > > > local > > > variable for that. This gives us an additional 8 bits, which we can > > > then use for the field width. > > > > > > To stay in 8 bytes, we need to do a little rearranging and make the > > > type member a bitfield as well. For consistency, change all the > > > members to bit fields. gcc doesn't generate much worse code with > > > these > > > changes (in fact, bloat-o-meter says we save 300 bytes - which I > > > think > > > is a little surprising). > > > > > > I didn't find a BUILD_BUG/compiletime_assertion/... which would > > > work > > > outside function context, so for now I just open-coded it. > > > > > > [1] http://thread.gmane.org/gmane.linux.kernel/2034835 > > > > Thanks for keeping at this Rasmus. > > This seems quite reasonable. > > I like most of the stuff here, though, Joe, can we avoid open-coded > BUILD_BUG_ON()?
Well we could just do
--- a/lib/vsprintf.c~lib-vsprintfc-expand-field_width-to-24-bits-fix +++ a/lib/vsprintf.c @@ -386,7 +386,6 @@ struct printf_spec { unsigned int base:8; /* number base, 8, 10 or 16 only */ signed int precision:16; /* # of digits/chars */ } __packed; -extern char __check_printf_spec[1-2*(sizeof(struct printf_spec) != 8)]; static noinline_for_stack char *number(char *buf, char *end, unsigned long long num, @@ -400,6 +399,8 @@ char *number(char *buf, char *end, unsig int i; bool is_zero = num == 0LL; + BUILD_BUG_ON(sizeof(struct printf_spec) != 8); + /* locase = 0 or 0x20. ORing digits or letters with 'locase' * produces same digits or (maybe lowercased) letters */ locase = (spec.flags & SMALL); Which is better than open-coding it, IMO.
I've been fiddling with a BUILD_BUG_ON which works outside functions using gcc's __COUNTER__ - something like
#define BBO(expr) typedef char __bbo##__COUNTER__[1-2*(!!expr)]
BBO(1 == 1); BBO(2 == 2);
but that comes out as
typedef char __bbo__COUNTER__[1-2*(!!1 == 1)]; typedef char __bbo__COUNTER__[1-2*(!!2 == 2)];
instead of
typedef char __bbo0[1-2*(!!1 == 1)]; typedef char __bbo1[1-2*(!!2 == 2)];
There's some trick here but I've forgotten what it is.
| |