Re: [patch net-next v4 01/10] flow_offload: Introduce offload of HW stats type

Edward Cree
2020-03-09T22:28:14Z

On 09/03/2020 21:36, Jakub Kicinski wrote:
> ...but using the type for fields which are very likely to contain
> values from outside of the enumeration seems confusing, IMHO.
It's the only way I can see to (a) ensure that the field is the correct
size and (b) document the connection between the field and the type.

Now, I'm not saying that using the enums actually buys any real type-
safety — the C standard enums are far too weak for that, being
basically ints with hair — but the mythical Sufficiently Smart
Compiler might be able to get some mileage out of it to issue better
diagnostics.

> Driver author can understandably try to simply handle all the values
> in a switch statement and be unpleasantly surprised.
In my experience, unenumerated enum values of this kind are fully
idiomatic C; and a driver author looking at the header file to see
what enumeration constants are defined will necessarily see all the
calls to BIT() and the bitwise-or construction of _ANY.
I'm not sure I believe a naïve switch() implementation is an
"understandable" error.

How about if we also rename the field "hw_stats_types", or squeeze
the word "mask" in there somewhere?

-ed