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: Devise macros to encapsulate (x & -x)
Date: Fri, 17 Nov 2023 11:17:43 +0100	[thread overview]
Message-ID: <08e6cb27d65250d109df0ef8a49dc80a@bugseng.com> (raw)

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

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


             reply	other threads:[~2023-11-17 10:18 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-11-17 10:17 Nicola Vetrini [this message]
2023-11-17 11:04 ` Devise macros to encapsulate (x & -x) 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

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=08e6cb27d65250d109df0ef8a49dc80a@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.