On Tue, Jul 04, 2023 at 05:07:17PM +0300, Dan Carpenter wrote:
> On Tue, Jul 04, 2023 at 04:38:41PM +0300, Leon Romanovsky wrote:
> > On Thu, Jun 29, 2023 at 09:07:37AM +0300, Dan Carpenter wrote:
> > > This code is trying to ensure that only the flags specified in the list
> > > are allowed. The problem is that ucmd->rx_hash_fields_mask is a u64 and
> > > the flags are an enum which is treated as a u32 in this context. That
> > > means the test doesn't check whether the highest 32 bits are zero.
> > >
> > > Fixes: 4d02ebd9bbbd ("IB/mlx4: Fix RSS hash fields restrictions")
> > > Signed-off-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
> > > ---
> > > The MLX4_IB_RX_HASH_INNER value is declared as
> > > "MLX4_IB_RX_HASH_INNER = 1ULL << 31," which suggests that it
> > > should be type ULL but that doesn't work. It will still be basically a
> > > u32. (Enum types are weird).
> >
> > Can you please elaborate more why enum left to be int? It is surprise to me.
>
> Enum types are not defined very strictly in C so it's up to the
> compiler.
>
> Clang, GCC and Sparse implement them in the same way. They default
> to u32 unless the values can't fit, then they become whatever type fits.
> So if you have a negative, it becomes an int or a big value changes the
> type to unsigned long.
It is worse than that, the standard has some wording that the
constants have to be 'int' so gcc makes most of those values 'int'
when it computes the | across them. There is some 'beyond C' behavior
here where gcc will make only the non-int representable constants
some larger type (ie MLX4_IB_RX_HASH_INNER is u32 and
MLX4_IB_RX_HASH_SRC_IPV4 is int)
This is totally un-intuitive that the type of the enum constants is
not the type of the enum itself (which is u32 in this case), but here
we are.
C23 finally fixes this by brining the C++ feature of explicitly typed
enums and then the enum and all the constants have a consistent,
specified, type.
But this is definately the right thing to do, I actually thought we
had a function specifically for doing this test becaue of how tricky ~
is...
Jason
