[PATCH] hwmon: submitting-patches: Explain race conditions caused by calculations in macros

[PATCH] hwmon: submitting-patches: Explain race conditions caused by calculations in macros

[Gui-Dong Han]

The current documentation advises against calculations in macros
primarily to avoid code obfuscation. It misses the risk of concurrency
issues.

Add a note explaining that macros evaluating arguments multiple times
can lead to race conditions when accessing shared data.

Link: https://lore.kernel.org/all/CALbr=LYJ_ehtp53HXEVkSpYoub+XYSTU8Rg=o1xxMJ8=5z8B-g@mail.gmail.com/
Signed-off-by: Gui-Dong Han <hanguidong02@gmail.com>
---
 Documentation/hwmon/submitting-patches.rst | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/Documentation/hwmon/submitting-patches.rst b/Documentation/hwmon/submitting-patches.rst
index 6482c4f137dc..7f7095951750 100644
--- a/Documentation/hwmon/submitting-patches.rst
+++ b/Documentation/hwmon/submitting-patches.rst
@@ -82,7 +82,10 @@ increase the chances of your change being accepted.
 * Avoid calculations in macros and macro-generated functions. While such macros
   may save a line or so in the source, it obfuscates the code and makes code
   review more difficult. It may also result in code which is more complicated
-  than necessary. Use inline functions or just regular functions instead.
+  than necessary. Such macros may also evaluate their arguments multiple times.
+  This leads to Time-of-Check to Time-of-Use (TOCTOU) race conditions when
+  accessing shared data without locking, for example when calculating values in
+  sysfs show functions. Use inline functions or just regular functions instead.
 
 * Limit the number of kernel log messages. In general, your driver should not
   generate an error message just because a runtime operation failed. Report
-- 
2.43.0

Re: [PATCH] hwmon: submitting-patches: Explain race conditions caused by calculations in macros

[David Laight]

On Wed,  3 Dec 2025 01:55:36 +0800
Gui-Dong Han <hanguidong02@gmail.com> wrote:

> The current documentation advises against calculations in macros
> primarily to avoid code obfuscation. It misses the risk of concurrency
> issues.
> 
> Add a note explaining that macros evaluating arguments multiple times
> can lead to race conditions when accessing shared data.
> 
> Link: https://lore.kernel.org/all/CALbr=LYJ_ehtp53HXEVkSpYoub+XYSTU8Rg=o1xxMJ8=5z8B-g@mail.gmail.com/
> Signed-off-by: Gui-Dong Han <hanguidong02@gmail.com>
> ---
>  Documentation/hwmon/submitting-patches.rst | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/hwmon/submitting-patches.rst b/Documentation/hwmon/submitting-patches.rst
> index 6482c4f137dc..7f7095951750 100644
> --- a/Documentation/hwmon/submitting-patches.rst
> +++ b/Documentation/hwmon/submitting-patches.rst
> @@ -82,7 +82,10 @@ increase the chances of your change being accepted.
>  * Avoid calculations in macros and macro-generated functions. While such macros
>    may save a line or so in the source, it obfuscates the code and makes code
>    review more difficult. It may also result in code which is more complicated
> -  than necessary. Use inline functions or just regular functions instead.
> +  than necessary. Such macros may also evaluate their arguments multiple times.
> +  This leads to Time-of-Check to Time-of-Use (TOCTOU) race conditions when
> +  accessing shared data without locking, for example when calculating values in
> +  sysfs show functions. Use inline functions or just regular functions instead.

That is only half the story.
#defines are fine - provided they are written properly.
The main issue isn't TOCTOU, but just calls like foo(*arg++).
It is important that side effects of arguments only happen once.
So it is important that #defines don't evaluate their arguments more than once.
That is the real issue.

If you are reading shared data without locks there are much bigger problems
if it really matters that you get a valid value (see READ_ONCE()).

	David

>  
>  * Limit the number of kernel log messages. In general, your driver should not
>    generate an error message just because a runtime operation failed. Report

Re: [PATCH] hwmon: submitting-patches: Explain race conditions caused by calculations in macros

[Guenter Roeck]

On Wed, Dec 03, 2025 at 01:55:36AM +0800, Gui-Dong Han wrote:
> The current documentation advises against calculations in macros
> primarily to avoid code obfuscation. It misses the risk of concurrency
> issues.
> 
> Add a note explaining that macros evaluating arguments multiple times
> can lead to race conditions when accessing shared data.
> 
> Link: https://lore.kernel.org/all/CALbr=LYJ_ehtp53HXEVkSpYoub+XYSTU8Rg=o1xxMJ8=5z8B-g@mail.gmail.com/
> Signed-off-by: Gui-Dong Han <hanguidong02@gmail.com>

Applied.

Side note: this is good enough for me. I'll be happy to accept a
separate patch with a more detailed explanation.

Thanks,
Guenter

> ---
>  Documentation/hwmon/submitting-patches.rst | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/hwmon/submitting-patches.rst b/Documentation/hwmon/submitting-patches.rst
> index 6482c4f137dc..7f7095951750 100644
> --- a/Documentation/hwmon/submitting-patches.rst
> +++ b/Documentation/hwmon/submitting-patches.rst
> @@ -82,7 +82,10 @@ increase the chances of your change being accepted.
>  * Avoid calculations in macros and macro-generated functions. While such macros
>    may save a line or so in the source, it obfuscates the code and makes code
>    review more difficult. It may also result in code which is more complicated
> -  than necessary. Use inline functions or just regular functions instead.
> +  than necessary. Such macros may also evaluate their arguments multiple times.
> +  This leads to Time-of-Check to Time-of-Use (TOCTOU) race conditions when
> +  accessing shared data without locking, for example when calculating values in
> +  sysfs show functions. Use inline functions or just regular functions instead.
>  
>  * Limit the number of kernel log messages. In general, your driver should not
>    generate an error message just because a runtime operation failed. Report