Date:   Sun, 12 Mar 2023 15:12:47 +0100 (CET)
From:   Julia Lawall <julia.lawall@inria.fr>
To:     Menna Mahmoud <eng.mennamahmoud.mm@gmail.com>
cc:     outreachy@lists.linux.dev, lars@metafoo.de,
        Michael.Hennerich@analog.com, jic23@kernel.org,
        gregkh@linuxfoundation.org, linux-iio@vger.kernel.org,
        linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] staging: iio: meter: enclose Macros with complex values
 in parentheses
In-Reply-To: <20230312133347.120944-1-eng.mennamahmoud.mm@gmail.com>
Message-ID: <alpine.DEB.2.22.394.2303121507450.2865@hadrien>
References: <20230312133347.120944-1-eng.mennamahmoud.mm@gmail.com>
User-Agent: Alpine 2.22 (DEB 394 2020-01-19)
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="8323329-1354038481-1678630367=:2865"
Precedence: bulk

  This message is in MIME format.  The first part should be readable text,
  while the remaining parts are likely unreadable without MIME-aware tools.

--8323329-1354038481-1678630367=:2865
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8BIT


On Sun, 12 Mar 2023, Menna Mahmoud wrote:

> enclose Macros with complex values in parentheses is especially useful
> in making macro definitions “safe” (so that they
> evaluate each operand exactly once).

enclose -> Enclose, and Macros -> macros

I don't understand the above comment though.  How does adding parentheses
around the body of a macro cause the operands to be evaluated only once?
And the macros that you have changed don't have any operands.

The value of adding parentheses is normally to ensure that the body of the
macro doesn't interact with the context in a weird way.  For example, you
could have

#define ADD 3 + 4

Then if you use your macro as 6 * ADD, you will end up evaluating
6 * 3 + 4, ie 18 + 4, when you might have expected 6 * 7.  The issue is
that * has higher precedence than +.

But I don't think that such a problem can arise with a cast expression, so
parentheses around it should not be necessary.

> this error reported by chechpatch.pl

this error is reported by checkpatch.

>
> "ERROR: Macros with complex values should be enclosed in parentheses"
>
> for ADE7854_SPI_SLOW, ADE7854_SPI_BURST and ADE7854_SPI_FAST
> macros and this error fixed by enclose these macros in parentheses.

The last two lines aren't needed.  One can easily see that from looking at
the patch.

julia

> Signed-off-by: Menna Mahmoud <eng.mennamahmoud.mm@gmail.com>
> ---
>  drivers/staging/iio/meter/ade7854.h | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/staging/iio/meter/ade7854.h b/drivers/staging/iio/meter/ade7854.h
> index 7a49f8f1016f..41eeedef569b 100644
> --- a/drivers/staging/iio/meter/ade7854.h
> +++ b/drivers/staging/iio/meter/ade7854.h
> @@ -139,9 +139,9 @@
>  #define ADE7854_MAX_RX    7
>  #define ADE7854_STARTUP_DELAY 1000
>
> -#define ADE7854_SPI_SLOW	(u32)(300 * 1000)
> -#define ADE7854_SPI_BURST	(u32)(1000 * 1000)
> -#define ADE7854_SPI_FAST	(u32)(2000 * 1000)
> +#define ADE7854_SPI_SLOW	((u32)(300 * 1000))
> +#define ADE7854_SPI_BURST	((u32)(1000 * 1000))
> +#define ADE7854_SPI_FAST	((u32)(2000 * 1000))
>
>  /**
>   * struct ade7854_state - device instance specific data
> --
> 2.34.1
>
>
>
--8323329-1354038481-1678630367=:2865--