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)
next 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.