2020-06-07 05:18:03

by Hu Haowen

[permalink] [raw]
Subject: [PATCH] net/mlx5: Add a missing macro undefinition

The macro ODP_CAP_SET_MAX is only used in function handle_hca_cap_odp()
in file main.c, so it should be undefined when there are no more uses
of it.

Signed-off-by: Hu Haowen <[email protected]>
---
drivers/net/ethernet/mellanox/mlx5/core/main.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/main.c b/drivers/net/ethernet/mellanox/mlx5/core/main.c
index df46b1fce3a7..1143297eccaa 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/main.c
@@ -489,6 +489,8 @@ static int handle_hca_cap_odp(struct mlx5_core_dev *dev, void *set_ctx)
ODP_CAP_SET_MAX(dev, dc_odp_caps.read);
ODP_CAP_SET_MAX(dev, dc_odp_caps.atomic);

+#undef ODP_CAP_SET_MAX
+
if (!do_set)
return 0;

--
2.25.1



2020-06-07 06:39:40

by Leon Romanovsky

[permalink] [raw]
Subject: Re: [PATCH] net/mlx5: Add a missing macro undefinition

On Sun, Jun 07, 2020 at 01:12:40PM +0800, Hu Haowen wrote:
> The macro ODP_CAP_SET_MAX is only used in function handle_hca_cap_odp()
> in file main.c, so it should be undefined when there are no more uses
> of it.
>
> Signed-off-by: Hu Haowen <[email protected]>
> ---
> drivers/net/ethernet/mellanox/mlx5/core/main.c | 2 ++
> 1 file changed, 2 insertions(+)

"should be undefined" is s little bit over statement, but overall
the patch is good.

Fixes: fca22e7e595f ("net/mlx5: ODP support for XRC transport is not enabled by default in FW")

Thanks,
Reviewed-by: Leon Romanovsky <[email protected]>

2020-06-07 06:59:06

by Hu Haowen

[permalink] [raw]
Subject: Re: [PATCH] net/mlx5: Add a missing macro undefinition


On 2020/6/7 2:36 PM, Leon Romanovsky wrote:
> On Sun, Jun 07, 2020 at 01:12:40PM +0800, Hu Haowen wrote:
>> The macro ODP_CAP_SET_MAX is only used in function handle_hca_cap_odp()
>> in file main.c, so it should be undefined when there are no more uses
>> of it.
>>
>> Signed-off-by: Hu Haowen <[email protected]>
>> ---
>> drivers/net/ethernet/mellanox/mlx5/core/main.c | 2 ++
>> 1 file changed, 2 insertions(+)
> "should be undefined" is s little bit over statement, but overall
> the patch is good.


Sorry for my strong tone, but my idea is that every macro which is
defined and used just in a single function, is supposed to be undefined
at the end of its final use, so that you won't get into trouble next
time if you define a macro with the same name as this one.


>
> Fixes: fca22e7e595f ("net/mlx5: ODP support for XRC transport is not enabled by default in FW")
>
> Thanks,
> Reviewed-by: Leon Romanovsky <[email protected]>

2020-06-07 23:39:54

by David Miller

[permalink] [raw]
Subject: Re: [PATCH] net/mlx5: Add a missing macro undefinition

From: Hu Haowen <[email protected]>
Date: Sun, 7 Jun 2020 14:55:33 +0800

>
> On 2020/6/7 2:36 PM, Leon Romanovsky wrote:
>> On Sun, Jun 07, 2020 at 01:12:40PM +0800, Hu Haowen wrote:
>>> The macro ODP_CAP_SET_MAX is only used in function
>>> handle_hca_cap_odp()
>>> in file main.c, so it should be undefined when there are no more uses
>>> of it.
>>>
>>> Signed-off-by: Hu Haowen <[email protected]>
>>> ---
>>> drivers/net/ethernet/mellanox/mlx5/core/main.c | 2 ++
>>> 1 file changed, 2 insertions(+)
>> "should be undefined" is s little bit over statement, but overall
>> the patch is good.
>
>
> Sorry for my strong tone, but my idea is that every macro which is
> defined and used just in a single function, is supposed to be
> undefined
> at the end of its final use, so that you won't get into trouble next
> time if you define a macro with the same name as this one.

The compiler would generate an error if that happened, so you would
not get into "trouble".

I fail to see the value in this change at all, sorry.

Really, what's the point?

Does it make the code harder to read? No.

Does it cause problems if you accidently want to use that macro name
again in the same compilation unit? No.

So I have yet to hear a valid "why" to make this kind of change and
I'd like to stop this set of cleanups before it gets out of control
and we have these ugly #undef statements all over the tree.

Thank you.