On 17.11.2023 12:15, Nicola Vetrini wrote:
> On 2023-11-17 12:04, Andrew Cooper wrote:
>> On 17/11/2023 10:17 am, 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_; \
>>> })
>>
>> Of course this was going to explode.
>>
>> This isn't even the first time an unwise attempt to do
>> single-evaluation
>> has needed to be reverted because it doesn't work with Integer Constant
>> Expressions.
>>
>> Switch it back to the first form. It's obviously a macro to begin
>> with,
>> and not likely to be used in cases that have side effects.
I guess Nicola's original mail was lacking some pieces. After the issue
with the statement expression form was pointed out, I never asked to
replace the existing (already committed, ...
> Actually no usages of either forms are yet committed, just the
> definition of the first form, so nothing needs to be reverted.
... with actual uses in MASK_EXTR() and MASK_INSR()) macro. Instead I
was suggesting to have a _second_ macro for use wherever Integer Constant
Expressions aren't the limiting factor. E.g. isolate_lsb(), deliberately
lower-case to look more like a function (and thus communicating that its
argument indeed is going to be evaluated only once, as would be the case
if the whole thing was a function).
Jan
