Re: [PATCH 2/2] bitmap: introduce for_each_set_bitrange

[Yury Norov]

On Fri, Jul 16, 2021 at 03:46:43PM +0200, Petr Mladek wrote:
> On Thu 2021-07-15 08:50:21, Yury Norov wrote:
> > On Fri, Jul 09, 2021 at 09:59:50AM -0400, Steven Rostedt wrote:
> > > On Thu,  8 Jul 2021 20:45:19 -0700
> > > Yury Norov <yury.norov@xxxxxxxxx> wrote:
> > > 
> > > > bitmap_list_string() is very ineffective when printing bitmaps with long
> > > > ranges of set bits because it calls find_next_bit for each bit. We can do
> > > > better by detecting ranges of set bits.
> > > > diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> > > > index 87acf66f0e4c..1ee54dace71e 100644
> > > > --- a/lib/vsprintf.c
> > > > +++ b/lib/vsprintf.c
> > > >  
> > > > -			buf = number(buf, end, rtop, default_dec_spec);
> > > > -		}
> > > > +	if (buf > start)
> > > > +		buf--;
> > > 
> > > If the above is to undo the last comma, please put back the first logic.
> > 
> > You're asking me to move part of the logic inside the loop which generally
> > should be avoided. Is there any particular reason to do this?
> 
> vsprintf() should write what is needed and keep the rest of the given
> buffer intact. There is even a test for this in the test_printf module.
> 
> I think that test_printf does not complain here because only a single
> character is used and it is later replaced by the trailing '\0'.
> 
> By other words, undoing the last comma does not cause visible problems
> in the end. But from vsprintf() point of view, it is a hack that does
> not trigger the warning only by chance. And it is better to avoid it.
> 
> Best Regards,
> Petr

Ah, OK. Thanks Petr.