Re: [PATCH 7/7] xen/device_tree: Fix MISRA C 2012 Rule 20.7 violations

[Xenia Ragiadakou <burzalodowa@xxxxxxxxx>]

Hi Jan,

On 8/22/22 12:59, Jan Beulich wrote:
> On 19.08.2022 21:43, Xenia Ragiadakou wrote:
> > In macros dt_for_each_property_node(), dt_for_each_device_node() and
> > dt_for_each_child_node(), add parentheses around the macro parameters that
> > have the arrow operator applied, to prevent against unintended expansions.
> Why is this relevant only when -> is used? For comparisons and the rhs of
> assignments it's as relevant, ad even for the lhs of assignments I doubt
> it can be generally omitted.
Yes, I agree with you but some older patches that I sent that were 
adding parentheses around the lhs of the assignments were not accepted 
and I thought that the rhs of the assignments as well these comparisons 
fall to the same category.
Personally, I would expect to see parentheses, also, around the macro 
parameters that are used as the lhs or the rhs of assignments, the 
operands of comparison or the arguments of a function.
Not only because they can prevent against unintentional bugs but because 
the parentheses help me to identify more easily the macro parameters 
when reading a macro definition. I totally understand that for other 
people parentheses may reduce readability.
> > --- a/xen/include/xen/device_tree.h
> > +++ b/xen/include/xen/device_tree.h
> > @@ -222,13 +222,13 @@ dt_find_interrupt_controller(const struct dt_device_match 
> > *matches);
> >   #define DT_ROOT_NODE_SIZE_CELLS_DEFAULT 1
> >   
> >   #define dt_for_each_property_node(dn, pp)                   \
> > -    for ( pp = dn->properties; pp != NULL; pp = pp->next )
> > +    for ( pp = (dn)->properties; pp != NULL; pp = (pp)->next )
> >   
> >   #define dt_for_each_device_node(dt, dn)                     \
> > -    for ( dn = dt; dn != NULL; dn = dn->allnext )
> > +    for ( dn = dt; dn != NULL; dn = (dn)->allnext )
> >   
> >   #define dt_for_each_child_node(dt, dn)                      \
> > -    for ( dn = dt->child; dn != NULL; dn = dn->sibling )
> > +    for ( dn = (dt)->child; dn != NULL; dn = (dn)->sibling )
> >   
> >   /* Helper to read a big number; size is in cells (not bytes) */
> >   static inline u64 dt_read_number(const __be32 *cell, int size)
--
Xenia