Re: [XEN PATCH][for-4.19 v4 1/8] xen/include: add macro ISOLATE_LOW_BIT

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Mon, 30 Oct 2023, Jan Beulich wrote:
> On 27.10.2023 15:34, Nicola Vetrini wrote:
> > --- a/xen/include/xen/macros.h
> > +++ b/xen/include/xen/macros.h
> > @@ -8,8 +8,14 @@
> >  #define DIV_ROUND(n, d) (((n) + (d) / 2) / (d))
> >  #define DIV_ROUND_UP(n, d) (((n) + (d) - 1) / (d))
> >  
> > -#define MASK_EXTR(v, m) (((v) & (m)) / ((m) & -(m)))
> > -#define MASK_INSR(v, m) (((v) * ((m) & -(m))) & (m))
> > +/*
> > + * Given an unsigned integer argument, expands to a mask where just the 
> > least
> > + * significant nonzero bit of the argument is set, or 0 if no bits are set.
> > + */
> > +#define ISOLATE_LOW_BIT(x) ((x) & -(x))
> 
> Not even considering future Misra changes (which aiui may require that
> anyway), this generalization of the macro imo demands that its argument
> now be evaluated only once.

Fur sure that would be an improvement, but I don't see a trivial way to
do it and this issue is also present today before the patch. I think it
would be better to avoid scope-creeping this patch as we are already at
v4 for something that was expected to be a trivial mechanical change. I
would rather review the fix as a separate patch, maybe sent by you as
you probably have a specific implementation in mind?


> Also another thought regarding the name: Would ISOLATE_LSB() be acceptable
> to everyone having voiced a view on the set of proposed names? It would be
> at least a little shorter ...

I would be OK with that