[PATCH] drivers/net/ethernet/sfc/: Simplify code

Subsystems: networking drivers, sfc network driver, the rest

STALE1720d

3 messages, 2 authors, 2021-11-22 · open the first message on its own page

[PATCH] drivers/net/ethernet/sfc/: Simplify code

From: Alejandro Colomar <hidden>
Date: 2021-11-20 20:15:36

That ternary operator has
the same exact code in both of the branches.

Unless there's some hidden magic in the condition,
there's no reason for it to be,
and it can be replaced
by the code in one of the branches.

That code has been untouched since it was added,
so there's no information in git about
why it was written that way.

Signed-off-by: Alejandro Colomar <redacted>
Cc: Edward Cree <ecree.xilinx@gmail.com>
Cc: Martin Habets <redacted>
Cc: netdev@vger.kernel.org
---
 drivers/net/ethernet/sfc/ethtool_common.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/sfc/ethtool_common.c b/drivers/net/ethernet/sfc/ethtool_common.c
index bd552c7dffcb..1b9e8b0afb3c 100644
--- a/drivers/net/ethernet/sfc/ethtool_common.c
+++ b/drivers/net/ethernet/sfc/ethtool_common.c
@@ -33,10 +33,7 @@ struct efx_sw_stat_desc {
 				get_stat_function) {			\
 	.name = #stat_name,						\
 	.source = EFX_ETHTOOL_STAT_SOURCE_##source_name,		\
-	.offset = ((((field_type *) 0) ==				\
-		      &((struct efx_##source_name *)0)->field) ?	\
-		    offsetof(struct efx_##source_name, field) :		\
-		    offsetof(struct efx_##source_name, field)),		\
+	.offset = offsetof(struct efx_##source_name, field),		\
 	.get_stat = get_stat_function,					\
 }
 
-- 
2.33.1

Re: [PATCH] drivers/net/ethernet/sfc/: Simplify code

From: Edward Cree <ecree.xilinx@gmail.com>
Date: 2021-11-22 16:17:37

On 20/11/2021 20:14, Alejandro Colomar wrote:
That ternary operator has
the same exact code in both of the branches.

Unless there's some hidden magic in the condition,
there's no reason for it to be,
and it can be replaced
by the code in one of the branches.

That code has been untouched since it was added,
so there's no information in git about
why it was written that way.

Signed-off-by: Alejandro Colomar <redacted>
Cc: Edward Cree <ecree.xilinx@gmail.com>
Cc: Martin Habets <redacted>
Cc: netdev@vger.kernel.org
I guess it's there for type-checking — essentially as an assert that
 field_type == typeof(efx_##source_name.field).  Probably when it was
 added there was no standard way to do this; now we could probably
 use <linux/typecheck.h> or some such.
The comment just above the macro does mention "with type-checking".

-ed

Re: [PATCH] drivers/net/ethernet/sfc/: Simplify code

From: Alejandro Colomar (man-pages) <hidden>
Date: 2021-11-22 17:19:23

Hi Edward,

On 11/22/21 17:17, Edward Cree wrote:
On 20/11/2021 20:14, Alejandro Colomar wrote:
quoted
That ternary operator has
the same exact code in both of the branches.

Unless there's some hidden magic in the condition,
there's no reason for it to be,
and it can be replaced
by the code in one of the branches.

That code has been untouched since it was added,
so there's no information in git about
why it was written that way.

Signed-off-by: Alejandro Colomar <redacted>
Cc: Edward Cree <ecree.xilinx@gmail.com>
Cc: Martin Habets <redacted>
Cc: netdev@vger.kernel.org
I guess it's there for type-checking — essentially as an assert that
  field_type == typeof(efx_##source_name.field).  Probably when it was
  added there was no standard way to do this; now we could probably
  use <linux/typecheck.h> or some such.
The comment just above the macro does mention "with type-checking".
Yes, that's why I suggested there was probably some black magic 
involved.  But I couldn't read it, though.

I suggest replacing it with a static_assert(__same_type()) thingy.

Cheers,
Alex


-- 
Alejandro Colomar
Linux man-pages comaintainer; http://www.kernel.org/doc/man-pages/
http://www.alejandro-colomar.es/
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help