netdev - RE: [Intel-wired-lan] [PATCH iwl-next v2 5/5] ixgbe: drop unnecessary constant casts to u16

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <IA3PR11MB898618236CA2EA46C0A861B7E54EA@IA3PR11MB8986.namprd11.prod.outlook.com>
Date: Tue, 8 Jul 2025 10:26:57 +0000
From: "Loktionov, Aleksandr" <aleksandr.loktionov@...el.com>
To: Jacek Kowalski <jacek@...ekk.info>, "Nguyen, Anthony L"
	<anthony.l.nguyen@...el.com>, "Kitszel, Przemyslaw"
	<przemyslaw.kitszel@...el.com>, Andrew Lunn <andrew+netdev@...n.ch>, "David
 S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, "Jakub
 Kicinski" <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, Simon Horman
	<horms@...nel.org>
CC: "intel-wired-lan@...ts.osuosl.org" <intel-wired-lan@...ts.osuosl.org>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: RE: [Intel-wired-lan] [PATCH iwl-next v2 5/5] ixgbe: drop unnecessary
 constant casts to u16



> -----Original Message-----
> From: Jacek Kowalski <jacek@...ekk.info>
> Sent: Tuesday, July 8, 2025 11:35 AM
> To: Loktionov, Aleksandr <aleksandr.loktionov@...el.com>; Nguyen,
> Anthony L <anthony.l.nguyen@...el.com>; Kitszel, Przemyslaw
> <przemyslaw.kitszel@...el.com>; Andrew Lunn <andrew+netdev@...n.ch>;
> David S. Miller <davem@...emloft.net>; Eric Dumazet
> <edumazet@...gle.com>; Jakub Kicinski <kuba@...nel.org>; Paolo Abeni
> <pabeni@...hat.com>; Simon Horman <horms@...nel.org>
> Cc: intel-wired-lan@...ts.osuosl.org; netdev@...r.kernel.org
> Subject: Re: [Intel-wired-lan] [PATCH iwl-next v2 5/5] ixgbe: drop
> unnecessary constant casts to u16
> 
> >> -	checksum = (u16)IXGBE_EEPROM_SUM - checksum;
> >> +	checksum = IXGBE_EEPROM_SUM - checksum;
> >>
> > Can't lead to different results, especially when:
> > checksum > IXGBE_EEPROM_SUM → the result becomes negative in int,
> and narrowing to u16 causes unexpected wraparound?
> >
> > With this patch you are changing the semantics of the code - from
> explicit 16bit arithmetic to full int implicit promotion which can be
> error-prone or compiler-dependent /* for different targets */.
> 
> 
> As far as I understand the C language does this by design - in the
> terms of C specification:
> 
> > If an int can represent all values of the original type (...), the
> value is converted to an int; otherwise, it is converted to an
> unsigned int. These are called the integer promotions. (see note)
> >
> > (note) The integer promotions are applied only: as part of the usual
> arithmetic conversions, (...)
> 
> 
> And subtraction semantics are:
> 
> > If both operands have arithmetic type, the usual arithmetic
> conversions are performed on them.
> 
> 
> 
> So there is no *16 bit arithmetic* - it is always done on integers
> (usually 32 bits).
> 
> Or have I missed something?
> 
> 
> Additionally I've checked AMD64, ARM and MIPS assembly output from GCC
> and clang on https://godbolt.org/z/GPsMxrWfe - both of the following
> snippets compile to exactly the same assembly:
> 
> #define NVM_SUM 0xBABA
> int test(int num) {
>     volatile unsigned short test = 0xFFFF;
>     unsigned short checksum = NVM_SUM - test;
>     return checksum;
> }
> 
> vs.:
> 
> #define NVM_SUM 0xBABA
> int test(int num) {
>     volatile unsigned short test = 0xFFFF;
>     unsigned short checksum = ((unsigned short)NVM_SUM) - test;
>     return checksum;
> }
> 
> --
> Best regards,
>   Jacek Kowalski

Thank you for the tests, I've copy pasted into Godbolt and' verified a doesn't of targets, the code is identical got gcc.
So, the change looks scary for the first glance, but GCC actually handles it the same way.

Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@...el.com>