On Wed, 13 Mar 2024 at 20:18, Jeff Johnson <quic_jjohnson@xxxxxxxxxxx> wrote: > > On 3/13/2024 9:58 AM, Kalle Valo wrote: > > Kalle Valo <kvalo@xxxxxxxxxx> writes: > > > >> Jeff Johnson <quic_jjohnson@xxxxxxxxxxx> writes: > >> > >>> On 3/13/2024 5:57 AM, Rameshkumar Sundaram wrote: > >>>> On 3/13/2024 3:23 AM, Jeff Johnson wrote: > >>>>> and guess we have to figure out how to suppress the ath12k-check issues with > >>>>> this macro > >>>> ath12k-check complains about the reuse of ah and index arguments which > >>>> may get evaluated multiple times if its an arithmetic expression, But > >>>> areas where we use the macro in our code aren't doing so. > >>>> Do you have any suggestions here ? or shall we go back and use this > >>>> for-loop inline. > >>> > >>> The macro makes sense -- we'll need to update the overrides in ath12k-check. > >> > >> IIRC it is possible to avoid variable reuse in macros with typeof() > >> operator (or something like that). I can't remember the details right > >> now but I think there are examples in the kernel code. > > > > Here's the GCC documentation with an example: > > > > https://gcc.gnu.org/onlinedocs/gcc/Typeof.html > > > > the problem here is that the macro actually writes to those arguments multiple > times, so we actually need to reuse the arguments > > the macro as defined exactly matches the semantics of other for_each macros in > the kernel, i.e. see in include/linux/list.h: > #define list_for_each(pos, head) \ > for (pos = (head)->next; !list_is_head(pos, (head)); pos = pos->next) > > what I don't understand is why list_for_each() doesn't trigger the same > checkpatch issues. even stranger is that if I copy that macro into ath12k then > I do see the same checkpatch issues: > CHECK: Macro argument reuse 'pos' - possible side-effects? > #998: FILE: drivers/net/wireless/ath/ath12k/core.h:998: > +#define list_for_each(pos, head) \ > + for (pos = (head)->next; !list_is_head(pos, (head)); pos = pos->next) > > CHECK: Macro argument reuse 'head' - possible side-effects? > #998: FILE: drivers/net/wireless/ath/ath12k/core.h:998: > +#define list_for_each(pos, head) \ > + for (pos = (head)->next; !list_is_head(pos, (head)); pos = pos->next) > > So I'm really confused since I don't see anything in checkpatch.pl that would > cause the behavior to change between macros in include/linux/list.h vs macros > in drivers/net/wireless/ath/ath12k/core.h The definition of the macro causes the complaint, not the usage of it. If you run checkpatch.pl on include/linux/list.h, you'll get the same output: $ ./scripts/checkpatch.pl --strict --file include/linux/list.h ... CHECK: Macro argument reuse 'pos' - possible side-effects? #686: FILE: include/linux/list.h:686: +#define list_for_each(pos, head) \ + for (pos = (head)->next; !list_is_head(pos, (head)); pos = pos->next) CHECK: Macro argument reuse 'head' - possible side-effects? #686: FILE: include/linux/list.h:686: +#define list_for_each(pos, head) \ + for (pos = (head)->next; !list_is_head(pos, (head)); pos = pos->next) ... Best Regards, Jonas
