Re: Devise macros to encapsulate (x & -x)

[Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>]

On 2023-11-17 11:17, Nicola Vetrini wrote:
> Hi all,
>
> As discussed in this thread [1], which is about complying with MISRA C 
> Rule 10.1,
> a macro was introduced to encapsulate a well-known construct:
>
> /*
>  * 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_LSB(x) ((x) & -(x))
>
> This macro has a gained some calls in the subsequent patches in that 
> thread, but concerns were raised around the fact that it would be 
> better to devise a macro that evaluates its argument only once. A 
> proposed solution is this (thanks to Jan Beulich):
> #define ISOLATE_LSB(x) ({ \
>      typeof(x) x_ = (x); \
>      x_ & -x_; \
> })
>
> However, it can't be used in all call sites that the previous macro 
> would have once that series is committed, as can be seen in [2]. 
> Therefore, while the implementation looks ok,
> a case has been made to have separate macros, of which the latter form 
> is preferred.
> The following points require some thought:
>
> - where the single evaluation macro should be placed?
>   One proposed location is xen/include/xen/bitops.h
> - is the proposed form actually the best, or maybe it could be an 
> inline function?
> Then testing can happen and a subsequent version of those uncommitted 
> patches introducing uses of either construct will be submitted, to 
> modify all the call sites only once in the commit history.
> Let me know what you think of this.
> Regards,
>
> [1] 
> https://lore.kernel.org/xen-devel/8a1313b3ab5ba6dd556cf37409e3b703@xxxxxxxxxxx/T/#mdeb510325e1acacb6477a88de8577e9e87351ba5
> [2] https://gitlab.com/xen-project/people/bugseng/xen/-/jobs/5423693947
I did a few tests; the only places where the first form can't be 
replaced are:
- inside BUILD_BUG_ON (two instances in x86/x86_64/mm.c)
- the definition of MASK_EXTR/MASK_INSR
- the definition of PDX_GROUP_COUNT

--
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)