[PATCH v4 01/12] wifi: ath12k: add multiple radio support in a single MAC HW un/register

[Jonas Gorski]

On Wed, 13 Mar 2024 at 20:18, Jeff Johnson <quic_jjohnson at quicinc.com> wrote:
>
> On 3/13/2024 9:58 AM, Kalle Valo wrote:
> > Kalle Valo <kvalo at kernel.org> writes:
> >
> >> Jeff Johnson <quic_jjohnson at quicinc.com> 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