diff mbox series

drivers/net/ethernet/sfc/: Simplify code

Message ID 20211120201425.799946-1-alx.manpages@gmail.com (mailing list archive)
State Awaiting Upstream
Delegated to: Netdev Maintainers
Headers show
Series drivers/net/ethernet/sfc/: Simplify code | expand

Checks

Context Check Description
netdev/tree_selection success Guessed tree name to be net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix warning Target tree name not specified in the subject
netdev/cover_letter success Single patches do not need cover letters
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers warning 2 maintainers not CCed: davem@davemloft.net kuba@kernel.org
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 11 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Alejandro Colomar Nov. 20, 2021, 8:14 p.m. UTC
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 <alx.manpages@gmail.com>
Cc: Edward Cree <ecree.xilinx@gmail.com>
Cc: Martin Habets <habetsm.xilinx@gmail.com>
Cc: netdev@vger.kernel.org
---
 drivers/net/ethernet/sfc/ethtool_common.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

Comments

Edward Cree Nov. 22, 2021, 4:17 p.m. UTC | #1
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 <alx.manpages@gmail.com>
> Cc: Edward Cree <ecree.xilinx@gmail.com>
> Cc: Martin Habets <habetsm.xilinx@gmail.com>
> 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
Alejandro Colomar Nov. 22, 2021, 5:19 p.m. UTC | #2
Hi Edward,

On 11/22/21 17:17, Edward Cree wrote:
> 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 <alx.manpages@gmail.com>
>> Cc: Edward Cree <ecree.xilinx@gmail.com>
>> Cc: Martin Habets <habetsm.xilinx@gmail.com>
>> 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
diff mbox series

Patch

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,					\
 }