- if ((old_vid != (u16)E1000_MNG_VLAN_NONE) &&
+ if ((old_vid != E1000_MNG_VLAN_NONE) &&
Ditto.
But more importantly, both Clang 20.1.7 W=1 builds (or at any rate,
builds with -Wtautological-constant-out-of-range-compare), and Smatch
complain that the comparison above is now always true because
E1000_MNG_VLAN_NONE is -1, while old_vid is unsigned.
You are right - I have missed that E1000_MNG_VLAN_NONE is negative.
Therefore (u16)E1000_MNG_VLAN_NONE has a side effect of causing a
wraparound.
It's even more interesting that (inadvertently) I have not made a
similar change in e1000e:
./drivers/net/ethernet/intel/e1000e/netdev.c:
if (adapter->mng_vlan_id != (u16)E1000_MNG_VLAN_NONE) {
Perhaps E1000_MNG_VLAN_NONE should be updated to be UINT16_MAX?
There's no UINT16_MAX in kernel as far as I know. I'd rather leave it as
it was or, if you insist on further refactoring, use either one of:
#define E1000_MNG_VLAN_NONE (u16)(~((u16) 0))
mimick ACPI: #define ACPI_UINT16_MAX (u16)(~((u16) 0))
#define E1000_MNG_VLAN_NONE ((u16)-1)
move the cast into the constant
#define E1000_MNG_VLAN_NONE 0xFFFF
use ready-made value
(parentheses left only due to the constant being "(-1)" and not "-1").
--
Best regards,
Jacek Kowalski
