Re: [XEN PATCH 08/11] x86/altcall: address violations of MISRA C Rule 20.7

[Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>]

On 2024-03-25 15:58, Jan Beulich wrote:
> On 25.03.2024 15:47, Nicola Vetrini wrote:
> > On 2024-03-25 10:38, Jan Beulich wrote:
> > > On 22.03.2024 17:01, Nicola Vetrini wrote:
> > > > MISRA C Rule 20.7 states: "Expressions resulting from the expansion
> > > > of macro parameters shall be enclosed in parentheses". Therefore, 
> > > > some
> > > > macro definitions should gain additional parentheses to ensure that
> > > > all
> > > > current and future users will be safe with respect to expansions 
> > > > that
> > > > can possibly alter the semantics of the passed-in macro parameter.
> > > >
> > > > No functional change.
> > > >
> > > > Signed-off-by: Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>
> > > Hmm. These macros are, at least in part, hard to read already. The
> > > added
> > > parentheses, while necessary when following the rule to the letter, 
> > > are
> > > making things worse, even if just slightly. I therefore have a 
> > > proposal
> > > /
> > > question:
> > >
> > > > --- a/xen/arch/x86/include/asm/alternative.h
> > > > +++ b/xen/arch/x86/include/asm/alternative.h
> > > > @@ -243,28 +243,28 @@ extern void alternative_branches(void);
> > > >
> > > >  #define alternative_vcall0(func) ({             \
> > > >      ALT_CALL_NO_ARG1;                           \
> > > > -    (void)sizeof(func());                       \
> > > > +    (void)sizeof((func)());                     \
> > > Like this, all that's touched here are (syntactical, but not real)
> > > function
> > > invocations. Function calls, like all postfix operators, are highest
> > > precedence. Hence by omitting parentheses in that case no breakage 
> > > can
> > > happen as a result: If the passed expression is another postfix one,
> > > that'll
> > > be evaluated first anyway. If any other expression is passed (aside
> > > primary
> > > ones, of course), that'll be evaluated afterwards only due to being
> > > lower
> > > precedence, irrespective of the presence/absence of parentheses.
> > >
> > > Therefore, where harmful to readability, can we perhaps leave postfix
> > > expressions alone?
> > While I can understand the benefits of this, and the reasoning on
> > postfix expressions, what about, for instance (modulo the actual
> > invocation, this is just an example)
> >
> > alternative_vcall0(2 + f) or similar scenarios?
> Any kind of such expression will break with alternative_callN()'s
> using of [addr] "i" (&(func)) as an asm() operand. Which clarifies
> that even of the postfix expressions only some (in particular not
> increment, decrement, and function call) could potentially be used
> with the altcall machinery.
>
> That said, you have a point in (indirectly) expressing that my
> earlier description wasn't quite right (as in: too generic, when
> it really needs tying to the specific case here).
>
> Jan
Ok, I see what you meant now. I'll deviate these two macros.

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