All of lore.kernel.org
 help / color / mirror / Atom feed
From: Nicola Vetrini <nicola.vetrini@bugseng.com>
To: Xen Devel <xen-devel@lists.xenproject.org>
Cc: Consulting <consulting@bugseng.com>,
	Stefano Stabellini <sstabellini@kernel.org>,
	Jbeulich <jbeulich@suse.com>,
	Andrew Cooper3 <andrew.cooper3@citrix.com>,
	George Dunlap <george.dunlap@citrix.com>,
	Julien Grall <julien@xen.org>, Wei Liu <wl@xen.org>,
	Roger Pau <roger.pau@citrix.com>,
	Bertrand Marquis <bertrand.marquis@arm.com>,
	Michal Orzel <michal.orzel@amd.com>
Subject: Re: Devise macros to encapsulate (x & -x)
Date: Fri, 17 Nov 2023 17:15:56 +0100	[thread overview]
Message-ID: <1bae6d3263ad8d8da75e13d48308875b@bugseng.com> (raw)
In-Reply-To: <08e6cb27d65250d109df0ef8a49dc80a@bugseng.com>

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@bugseng.com/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)


      parent reply	other threads:[~2023-11-17 16:16 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-11-17 10:17 Devise macros to encapsulate (x & -x) Nicola Vetrini
2023-11-17 11:04 ` Andrew Cooper
2023-11-17 11:15   ` Nicola Vetrini
2023-11-17 11:37     ` Jan Beulich
2023-11-18  2:46   ` Stefano Stabellini
2023-11-20 11:16     ` Julien Grall
2023-11-22  1:43       ` Stefano Stabellini
2023-11-22  8:07         ` Jan Beulich
2023-11-22  9:27           ` Nicola Vetrini
2023-11-22 21:22             ` Stefano Stabellini
2023-11-17 11:39 ` Jan Beulich
2023-11-17 16:12   ` Nicola Vetrini
2023-11-17 16:15 ` Nicola Vetrini [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1bae6d3263ad8d8da75e13d48308875b@bugseng.com \
    --to=nicola.vetrini@bugseng.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=bertrand.marquis@arm.com \
    --cc=consulting@bugseng.com \
    --cc=george.dunlap@citrix.com \
    --cc=jbeulich@suse.com \
    --cc=julien@xen.org \
    --cc=michal.orzel@amd.com \
    --cc=roger.pau@citrix.com \
    --cc=sstabellini@kernel.org \
    --cc=wl@xen.org \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.