[PATCH] doc/checkpatch: Add description to MACRO_ARG_REUSE

[PATCH] doc/checkpatch: Add description to MACRO_ARG_REUSE

[Martin Fernandez]

Add a description to the MACRO_ARG_REUSE check.

I feel like this is also a good place to put a workaround although I'm
not sure if there is a cannonical way to solve those kinds of issues.

Signed-off-by: Martin Fernandez <martin.fernandez@eclypsium.com>
---
 Documentation/dev-tools/checkpatch.rst | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/Documentation/dev-tools/checkpatch.rst b/Documentation/dev-tools/checkpatch.rst
index b52452bc2963..43fa99f188f5 100644
--- a/Documentation/dev-tools/checkpatch.rst
+++ b/Documentation/dev-tools/checkpatch.rst
@@ -759,6 +759,21 @@ Indentation and Line Breaks
 Macros, Attributes and Symbols
 ------------------------------
 
+  **ARG_REUSE**
+    Using the same argument multiple times in the macro definition
+    would lead to unwanted side-effects.
+
+    For example, given a `min` macro defined like::
+
+     #define min(x, y)  ((x) < (y) ? (x) : (y))
+
+    If you call it with `min(foo(x), 0)` would expand to::
+
+     foo(x) < 0 ? foo(x) : 0
+
+    If `foo` have side-effects or it's an expensive calculation the
+    results might not be what the user inteded.
+
   **ARRAY_SIZE**
     The ARRAY_SIZE(foo) macro should be preferred over
     sizeof(foo)/sizeof(foo[0]) for finding number of elements in an
-- 
2.30.2

Re: [PATCH] doc/checkpatch: Add description to MACRO_ARG_REUSE

[Randy Dunlap]

On 7/1/22 15:20, Martin Fernandez wrote:
> Add a description to the MACRO_ARG_REUSE check.
> 
> I feel like this is also a good place to put a workaround although I'm
> not sure if there is a cannonical way to solve those kinds of issues.

                         canonical


The usual way in the kernel is to declare a local _x and local _y (for your
example below).  See how it is done in include/linux/minmax.h for min_not_zero().
> 
> Signed-off-by: Martin Fernandez <martin.fernandez@eclypsium.com>
> ---
>  Documentation/dev-tools/checkpatch.rst | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/Documentation/dev-tools/checkpatch.rst b/Documentation/dev-tools/checkpatch.rst
> index b52452bc2963..43fa99f188f5 100644
> --- a/Documentation/dev-tools/checkpatch.rst
> +++ b/Documentation/dev-tools/checkpatch.rst
> @@ -759,6 +759,21 @@ Indentation and Line Breaks
>  Macros, Attributes and Symbols
>  ------------------------------
>  
> +  **ARG_REUSE**
> +    Using the same argument multiple times in the macro definition
> +    would lead to unwanted side-effects.
> +
> +    For example, given a `min` macro defined like::
> +
> +     #define min(x, y)  ((x) < (y) ? (x) : (y))
> +
> +    If you call it with `min(foo(x), 0)` would expand to::
> +
> +     foo(x) < 0 ? foo(x) : 0
> +
> +    If `foo` have side-effects or it's an expensive calculation the
> +    results might not be what the user inteded.

                                          intended.

> +
>    **ARRAY_SIZE**
>      The ARRAY_SIZE(foo) macro should be preferred over
>      sizeof(foo)/sizeof(foo[0]) for finding number of elements in an

thanks.
-- 
~Randy

Re: [PATCH] doc/checkpatch: Add description to MACRO_ARG_REUSE

[Martin Fernandez]

On 7/1/22, Randy Dunlap <rdunlap@infradead.org> wrote:
>
>
> On 7/1/22 15:20, Martin Fernandez wrote:
>> Add a description to the MACRO_ARG_REUSE check.
>>
>> I feel like this is also a good place to put a workaround although I'm
>> not sure if there is a cannonical way to solve those kinds of issues.
>
>                          canonical
>
>
> The usual way in the kernel is to declare a local _x and local _y (for your
> example below).  See how it is done in include/linux/minmax.h for
> min_not_zero().
>>
>> Signed-off-by: Martin Fernandez <martin.fernandez@eclypsium.com>
>> ---
>>  Documentation/dev-tools/checkpatch.rst | 15 +++++++++++++++
>>  1 file changed, 15 insertions(+)
>>
>> diff --git a/Documentation/dev-tools/checkpatch.rst
>> b/Documentation/dev-tools/checkpatch.rst
>> index b52452bc2963..43fa99f188f5 100644
>> --- a/Documentation/dev-tools/checkpatch.rst
>> +++ b/Documentation/dev-tools/checkpatch.rst
>> @@ -759,6 +759,21 @@ Indentation and Line Breaks
>>  Macros, Attributes and Symbols
>>  ------------------------------
>>
>> +  **ARG_REUSE**
>> +    Using the same argument multiple times in the macro definition
>> +    would lead to unwanted side-effects.
>> +
>> +    For example, given a `min` macro defined like::
>> +
>> +     #define min(x, y)  ((x) < (y) ? (x) : (y))
>> +
>> +    If you call it with `min(foo(x), 0)` would expand to::
>> +
>> +     foo(x) < 0 ? foo(x) : 0
>> +
>> +    If `foo` have side-effects or it's an expensive calculation the
>> +    results might not be what the user inteded.
>
>                                           intended.
>
>> +
>>    **ARRAY_SIZE**
>>      The ARRAY_SIZE(foo) macro should be preferred over
>>      sizeof(foo)/sizeof(foo[0]) for finding number of elements in an

Thank you for the feedback, I'll send a new one right away!

> thanks.
> --
> ~Randy
>