On Tue, Jan 26, 2021 at 11:16:25PM +0200, Andy Shevchenko wrote:
> On Tue, Jan 26, 2021 at 12:11:36PM -0500, Paul Gortmaker wrote:
> > This will reduce parameter passing and enable using nbits as part
> > of future dynamic region parameter parsing.
...
> > struct region r;
> > long ret;
> >
> > - bitmap_zero(maskp, nmaskbits);
> > + r.nbits = nmaskbits;
Alternatively (though I personally don't prefer it) you can clarify in the
definition block the initial values.
struct region r = { .nbist = nmaskbits };
> > + bitmap_zero(maskp, r.nbits);
>
> This sounds not right from style perspective.
> You have completely uninitialized r on stack, then you assign only one value
> for immediate use here and...
>
> > while (buf) {
> > buf = bitmap_find_region(buf);
> > @@ -655,7 +656,7 @@ int bitmap_parselist(const char *buf, unsigned long
> > *maskp, int nmaskbits)
> > if (ret)
> > return ret;
> >
> > - ret = bitmap_set_region(&r, maskp, nmaskbits);
> > + ret = bitmap_set_region(&r, maskp);
>
> ...hiding this fact here. Which I would expect that &r may be rewritten here.
>
> I would leave these unchanged and simple assign the value in
> bitmap_set_region().
--
With Best Regards,
Andy Shevchenko
