Rameshkumar Sundaram <quic_ramess@xxxxxxxxxxx> writes: > On 3/14/2024 1:26 AM, Jonas Gorski wrote: >> 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 >>>> > Thanks Kalle for the references, as Jeff mentioned below, we need to > reuse the arguments since we write to ar and index arguments on each > iteration. > > Defining local vars using typeof() without limiting their scope (since > we are defining a for_each iterator{}) leads other issues like > redefinition of variables in functions where we use this macro more > than once :( > > Also even if we somehow manage to convince check-patch, we'll still > end up evaluating index and ar arguments in every iteration of loop. > This just gives an impression to check-patch that the macro is unsafe > (although logically its not). > Experts, what is the standard we should follow here. Please suggest. Yeah, typeof() won't help here as we can't create a local variable. Or at least I can't come up way to do that safely, ideas very welcome. I think it's just best to ignore the checkpatch warning for now, unless better proposals come up. ath12k-check has functionality to ignore specific warnings (see checkpatch_filter array), I can add this warning to the array. -- https://patchwork.kernel.org/project/linux-wireless/list/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
