On Fri, 26 Aug 2022, Jan Beulich wrote: > On 26.08.2022 09:58, Xenia Ragiadakou wrote: > > On 8/26/22 09:21, Jan Beulich wrote: > >> On 25.08.2022 20:09, Stefano Stabellini wrote: > >>> But first, let's confirm whether this change: > >>> > >>> > >>> #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 ) > >>> > >>> > >>> is sufficient to make the violation go away in Eclair or cppcheck. I am > >>> assuming it is not sufficient, but let's confirm. > >> > >> Well, even if for the lhs of assignments there was an exception, this > >> still wouldn't be sufficient. The minimum needed is > >> > >> #define dt_for_each_property_node(dn, pp) \ > >> for ( pp = (dn)->properties; (pp) != NULL; pp = (pp)->next )
Thank you for noticing this > > If pp is assumed to be a valid lvalue, then why it is needed to add > > parentheses here (pp) != NULL ? > > Because in one expression is doesn't matter that in another expression > the same identifier is used as the lhs of an assignment. Whether > parentheses are needed should solely depend on the operators in use, > not any further context. This is the problem with going with a more sophisticated version of the rule: it is not immediately obvious any longer. I have to read this explanation three times to appreciate what it means, I don't think a new contributor would really have any chances of getting this right, especially as cppcheck/Eclair wouldn't be able to help them either.
