diff mbox series

[XEN,04/10] xen/public: address violations of MISRA C Rule 20.7

Message ID 0cdc4dc2fcad699a2274277b32de3ee0207d5a2d.1709219010.git.nicola.vetrini@bugseng.com (mailing list archive)
State Superseded
Headers show
Series address some violations of MISRA C Rule 20.7 | expand

Commit Message

Nicola Vetrini Feb. 29, 2024, 3:27 p.m. UTC
MISRA C Rule 20.7 states: "Expressions resulting from the expansion
of macro parameters shall be enclosed in parentheses". Therefore
the macro XEN_DEFINE_UUID_ should wrap its parameters in parentheses.

No functional change.

Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
---
 xen/include/public/xen.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Jan Beulich Feb. 29, 2024, 4:40 p.m. UTC | #1
On 29.02.2024 16:27, Nicola Vetrini wrote:
> --- a/xen/include/public/xen.h
> +++ b/xen/include/public/xen.h
> @@ -988,7 +988,7 @@ typedef struct {
>        ((b) >>  8) & 0xFF, ((b) >>  0) & 0xFF,                           \
>        ((c) >>  8) & 0xFF, ((c) >>  0) & 0xFF,                           \
>        ((d) >>  8) & 0xFF, ((d) >>  0) & 0xFF,                           \
> -                e1, e2, e3, e4, e5, e6}}
> +                (e1), (e2), (e3), (e4), (e5), (e6)}}

Why? Wasn't it agreed already that long macro arguments passed on
(no matter whether to a function, a macro, or like used here) don't
need parenthesizing?

Jan
Nicola Vetrini Feb. 29, 2024, 4:49 p.m. UTC | #2
On 2024-02-29 17:40, Jan Beulich wrote:
> On 29.02.2024 16:27, Nicola Vetrini wrote:
>> --- a/xen/include/public/xen.h
>> +++ b/xen/include/public/xen.h
>> @@ -988,7 +988,7 @@ typedef struct {
>>        ((b) >>  8) & 0xFF, ((b) >>  0) & 0xFF,                         
>>   \
>>        ((c) >>  8) & 0xFF, ((c) >>  0) & 0xFF,                         
>>   \
>>        ((d) >>  8) & 0xFF, ((d) >>  0) & 0xFF,                         
>>   \
>> -                e1, e2, e3, e4, e5, e6}}
>> +                (e1), (e2), (e3), (e4), (e5), (e6)}}
> 
> Why? Wasn't it agreed already that long macro arguments passed on
> (no matter whether to a function, a macro, or like used here) don't
> need parenthesizing?
> 

That applies to all outermost macro invocations, but not to the 
innermost one. If you want also aggregate initalizers to be deviated, 
that could be done (provided that the macro arg is not included in some 
expression, such as "{..., e1 + 1, ...}"
Stefano Stabellini Feb. 29, 2024, 10:49 p.m. UTC | #3
On Thu, 29 Feb 2024, Nicola Vetrini wrote:
> On 2024-02-29 17:40, Jan Beulich wrote:
> > On 29.02.2024 16:27, Nicola Vetrini wrote:
> > > --- a/xen/include/public/xen.h
> > > +++ b/xen/include/public/xen.h
> > > @@ -988,7 +988,7 @@ typedef struct {
> > >        ((b) >>  8) & 0xFF, ((b) >>  0) & 0xFF,                           \
> > >        ((c) >>  8) & 0xFF, ((c) >>  0) & 0xFF,                           \
> > >        ((d) >>  8) & 0xFF, ((d) >>  0) & 0xFF,                           \
> > > -                e1, e2, e3, e4, e5, e6}}
> > > +                (e1), (e2), (e3), (e4), (e5), (e6)}}
> > 
> > Why? Wasn't it agreed already that long macro arguments passed on
> > (no matter whether to a function, a macro, or like used here) don't
> > need parenthesizing?
> > 
> 
> That applies to all outermost macro invocations, but not to the innermost one.

I don't understand what you mean. Maybe a couple of trivial examples
would help.


> If you want also aggregate initalizers to be deviated, that could be done
> (provided that the macro arg is not included in some expression, such as
> "{..., e1 + 1, ...}"

My gut feeling tells me that probably this is what we want but I'd
rather first understand exactly what you meant above
Jan Beulich March 1, 2024, 7:54 a.m. UTC | #4
On 29.02.2024 17:49, Nicola Vetrini wrote:
> On 2024-02-29 17:40, Jan Beulich wrote:
>> On 29.02.2024 16:27, Nicola Vetrini wrote:
>>> --- a/xen/include/public/xen.h
>>> +++ b/xen/include/public/xen.h
>>> @@ -988,7 +988,7 @@ typedef struct {
>>>        ((b) >>  8) & 0xFF, ((b) >>  0) & 0xFF,                         
>>>   \
>>>        ((c) >>  8) & 0xFF, ((c) >>  0) & 0xFF,                         
>>>   \
>>>        ((d) >>  8) & 0xFF, ((d) >>  0) & 0xFF,                         
>>>   \
>>> -                e1, e2, e3, e4, e5, e6}}
>>> +                (e1), (e2), (e3), (e4), (e5), (e6)}}
>>
>> Why? Wasn't it agreed already that long macro arguments passed on
>> (no matter whether to a function, a macro, or like used here) don't
>> need parenthesizing?
>>
> 
> That applies to all outermost macro invocations, but not to the 
> innermost one. If you want also aggregate initalizers to be deviated, 
> that could be done (provided that the macro arg is not included in some 
> expression, such as "{..., e1 + 1, ...}"

Sure, this obviously needs to be "{..., (e1) + 1, ...}" then. But yes,
the full scope of the underlying pattern ought to be excluded from
needing (pointless) parentheses added. Even in a plain comma expression
you don't need them - to pass in a comma expression the invocation site
of such a macro would then need to parenthesize the respective operand
(or else it would be two separate operands).

The one case we're leaving aside here anyway is use of odd things like

#define M a, b

and then passing M into another macro.

Jan
Nicola Vetrini March 5, 2024, 10:21 a.m. UTC | #5
On 2024-02-29 23:49, Stefano Stabellini wrote:
> On Thu, 29 Feb 2024, Nicola Vetrini wrote:
>> On 2024-02-29 17:40, Jan Beulich wrote:
>> > On 29.02.2024 16:27, Nicola Vetrini wrote:
>> > > --- a/xen/include/public/xen.h
>> > > +++ b/xen/include/public/xen.h
>> > > @@ -988,7 +988,7 @@ typedef struct {
>> > >        ((b) >>  8) & 0xFF, ((b) >>  0) & 0xFF,                           \
>> > >        ((c) >>  8) & 0xFF, ((c) >>  0) & 0xFF,                           \
>> > >        ((d) >>  8) & 0xFF, ((d) >>  0) & 0xFF,                           \
>> > > -                e1, e2, e3, e4, e5, e6}}
>> > > +                (e1), (e2), (e3), (e4), (e5), (e6)}}
>> >
>> > Why? Wasn't it agreed already that long macro arguments passed on
>> > (no matter whether to a function, a macro, or like used here) don't
>> > need parenthesizing?
>> >
>> 
>> That applies to all outermost macro invocations, but not to the 
>> innermost one.
> 
> I don't understand what you mean. Maybe a couple of trivial examples
> would help.
> 
> 
>> If you want also aggregate initalizers to be deviated, that could be 
>> done
>> (provided that the macro arg is not included in some expression, such 
>> as
>> "{..., e1 + 1, ...}"
> 

Sorry for the late reply. This is the current state:

#define N(x) somestruct var = {..., x, ...}; // <- not deviated, 
violation here
#define M(x) N(x) // <- deviated, no violation here

...

M(0xff);

The violation is resolved by {..., (x), ...} or by saying that when "x" 
is a whole expression in its fully expanded form, then we allow it not 
to be needlessly parenthesized, as Jan requested (unless I misunderstood 
his reply). In that case, the only this that would still give a 
violation in the above setting is questionable patterns such as

#define Q(x) x, x



> My gut feeling tells me that probably this is what we want but I'd
> rather first understand exactly what you meant above
Jan Beulich March 5, 2024, 10:26 a.m. UTC | #6
On 05.03.2024 11:21, Nicola Vetrini wrote:
> On 2024-02-29 23:49, Stefano Stabellini wrote:
>> On Thu, 29 Feb 2024, Nicola Vetrini wrote:
>>> On 2024-02-29 17:40, Jan Beulich wrote:
>>>> On 29.02.2024 16:27, Nicola Vetrini wrote:
>>>>> --- a/xen/include/public/xen.h
>>>>> +++ b/xen/include/public/xen.h
>>>>> @@ -988,7 +988,7 @@ typedef struct {
>>>>>        ((b) >>  8) & 0xFF, ((b) >>  0) & 0xFF,                           \
>>>>>        ((c) >>  8) & 0xFF, ((c) >>  0) & 0xFF,                           \
>>>>>        ((d) >>  8) & 0xFF, ((d) >>  0) & 0xFF,                           \
>>>>> -                e1, e2, e3, e4, e5, e6}}
>>>>> +                (e1), (e2), (e3), (e4), (e5), (e6)}}
>>>>
>>>> Why? Wasn't it agreed already that long macro arguments passed on
>>>> (no matter whether to a function, a macro, or like used here) don't
>>>> need parenthesizing?
>>>>
>>>
>>> That applies to all outermost macro invocations, but not to the 
>>> innermost one.
>>
>> I don't understand what you mean. Maybe a couple of trivial examples
>> would help.
>>
>>
>>> If you want also aggregate initalizers to be deviated, that could be 
>>> done
>>> (provided that the macro arg is not included in some expression, such 
>>> as
>>> "{..., e1 + 1, ...}"
>>
> 
> Sorry for the late reply. This is the current state:
> 
> #define N(x) somestruct var = {..., x, ...}; // <- not deviated, 
> violation here
> #define M(x) N(x) // <- deviated, no violation here
> 
> ...
> 
> M(0xff);
> 
> The violation is resolved by {..., (x), ...} or by saying that when "x" 
> is a whole expression in its fully expanded form, then we allow it not 
> to be needlessly parenthesized, as Jan requested (unless I misunderstood 
> his reply).

Well, the thing I continue to have trouble with is "fully expanded form".
That's not the criteria I'd like to see applied. To me all depends on how
the macro is written, not what uses of the macro expand to.

> In that case, the only this that would still give a 
> violation in the above setting is questionable patterns such as
> 
> #define Q(x) x, x

Right.

#define Q(x) (x, x)

ought to be okay though, rule-wise, no matter that it's questionable too.

Jan
Nicola Vetrini March 5, 2024, 4:17 p.m. UTC | #7
On 2024-03-05 11:26, Jan Beulich wrote:
> On 05.03.2024 11:21, Nicola Vetrini wrote:
>> On 2024-02-29 23:49, Stefano Stabellini wrote:
>>> On Thu, 29 Feb 2024, Nicola Vetrini wrote:
>>>> On 2024-02-29 17:40, Jan Beulich wrote:
>>>>> On 29.02.2024 16:27, Nicola Vetrini wrote:
>>>>>> --- a/xen/include/public/xen.h
>>>>>> +++ b/xen/include/public/xen.h
>>>>>> @@ -988,7 +988,7 @@ typedef struct {
>>>>>>        ((b) >>  8) & 0xFF, ((b) >>  0) & 0xFF,                     
>>>>>>       \
>>>>>>        ((c) >>  8) & 0xFF, ((c) >>  0) & 0xFF,                     
>>>>>>       \
>>>>>>        ((d) >>  8) & 0xFF, ((d) >>  0) & 0xFF,                     
>>>>>>       \
>>>>>> -                e1, e2, e3, e4, e5, e6}}
>>>>>> +                (e1), (e2), (e3), (e4), (e5), (e6)}}
>>>>> 
>>>>> Why? Wasn't it agreed already that long macro arguments passed on
>>>>> (no matter whether to a function, a macro, or like used here) don't
>>>>> need parenthesizing?
>>>>> 
>>>> 
>>>> That applies to all outermost macro invocations, but not to the
>>>> innermost one.
>>> 
>>> I don't understand what you mean. Maybe a couple of trivial examples
>>> would help.
>>> 
>>> 
>>>> If you want also aggregate initalizers to be deviated, that could be
>>>> done
>>>> (provided that the macro arg is not included in some expression, 
>>>> such
>>>> as
>>>> "{..., e1 + 1, ...}"
>>> 
>> 
>> Sorry for the late reply. This is the current state:
>> 
>> #define N(x) somestruct var = {..., x, ...}; // <- not deviated,
>> violation here
>> #define M(x) N(x) // <- deviated, no violation here
>> 
>> ...
>> 
>> M(0xff);
>> 
>> The violation is resolved by {..., (x), ...} or by saying that when 
>> "x"
>> is a whole expression in its fully expanded form, then we allow it not
>> to be needlessly parenthesized, as Jan requested (unless I 
>> misunderstood
>> his reply).
> 
> Well, the thing I continue to have trouble with is "fully expanded 
> form".
> That's not the criteria I'd like to see applied. To me all depends on 
> how
> the macro is written, not what uses of the macro expand to.
> 

Sure.

>> In that case, the only this that would still give a
>> violation in the above setting is questionable patterns such as
>> 
>> #define Q(x) x, x
> 
> Right.
> 
> #define Q(x) (x, x)
> 
> ought to be okay though, rule-wise, no matter that it's questionable 
> too.
> 
> Jan
diff mbox series

Patch

diff --git a/xen/include/public/xen.h b/xen/include/public/xen.h
index b47d48d0e2d6..fa23080bd7af 100644
--- a/xen/include/public/xen.h
+++ b/xen/include/public/xen.h
@@ -988,7 +988,7 @@  typedef struct {
       ((b) >>  8) & 0xFF, ((b) >>  0) & 0xFF,                           \
       ((c) >>  8) & 0xFF, ((c) >>  0) & 0xFF,                           \
       ((d) >>  8) & 0xFF, ((d) >>  0) & 0xFF,                           \
-                e1, e2, e3, e4, e5, e6}}
+                (e1), (e2), (e3), (e4), (e5), (e6)}}
 
 #if defined(__STDC_VERSION__) ? __STDC_VERSION__ >= 199901L : defined(__GNUC__)
 #define XEN_DEFINE_UUID(a, b, c, d, e1, e2, e3, e4, e5, e6)             \