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

From: Tejun Heo
Date: Thu Sep 10 2015 - 10:44:23 EST


Hello,

On Thu, Sep 10, 2015 at 01:17:36AM -0700, Joe Perches wrote:
> > It also won't work for the case at hand if/when the actual bitmap ever
> > gets a bit set beyond S16_MAX.
>
> But at least it should work for the bitmap sized <= S16_MAX
> which should be the majority of uses.

The failure mode is too subtle and this is a utility function which is
too fundamental. I think we really should just get it working
properly.

> > 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)
>
> Yes, but it still couldn't work without the ability to have
> large output buffers and printk doesn't support that.

printk overrunning its output buffer is fine. We've always had that
and while we probably should do a better job of indicating those
events, when the output line runs off that long, people (and it's
usually human minds that process printk outputs) already kinda suspect
that.

Long bitmaps don't mean long output and silently formatting the wrong
output sounds like a pretty bad idea and that because we can't make
the width field 32bit? It doesn't make any sense.

> seq_printf might have some performance issue with it too as
> it would repetitively try to emit, fail, and grow the buffer.

That's kinda beside the point here.

Thanks.

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