On Fri, 17 May 2024 13:57:58 +1000
David Gibson <david(a)gibson.dropbear.id.au> wrote:
On Thu, May 16, 2024 at 11:30:58AM +0200, Stefano
Brivio wrote:
On Tue, 14 May 2024 11:03:19 +1000
David Gibson <david(a)gibson.dropbear.id.au> wrote:
[...]
/**
* struct flow_common - Common fields for packet flows
+ * @state: State of the flow table entry
* @type: Type of packet flow
*/
struct flow_common {
+ uint8_t state;
In this case, I would typically do
(
https://seitan.rocks/seitan/tree/common/gluten.h?id=5a9302bab9c9bb3d1577f04…):
#ifdef __GNUC__
enum flow_state state:8;
#else
uint8_t state;
#endif
I don't object to that, but I have two questions
- What's the advantage to using the explicit enum? Is that for the
benefit of static checkers and/or compiler diagnostics?
Yes: if we assign a value that's not in the enum, I expect static
checkers to complain. But also for humans: even with that ifdef, a
reader would know right away what values that might have.
AFAIK C
itself doesn't really treat enums any differently to integer
types.
Right.
- What's the need for GNUC? Are enum
bitfields a gnu extension?
They're rather permitted by gcc's interpretation of the standard:
https://gcc.gnu.org/onlinedocs/gcc-14.1.0/gcc/Structures-unions-enumeration…
Allowable bit-field types other than _Bool, signed int, and unsigned
int (C99 and C11 6.7.2.1).
Other integer types, such as long int, and enumerated types are permitted
even in strictly conforming mode.
but C11 says (6.7.2.1):
A bit-field shall have a type that is a qualified or unqualified
version of _Bool, signed int, unsigned int, or some other
implementation-defined type. It is implementation-defined whether
atomic types are permitted.
Is an enum a "version" of those? Maybe not. That was at least the
interpretation adopted by older gcc versions, up to 4.7.4:
https://gcc.gnu.org/onlinedocs/gcc-4.7.4/gcc/Structures-unions-enumerations…
Allowable bit-field types other than _Bool, signed int, and unsigned
int (C99 6.7.2.1).
No other types are permitted in strictly conforming mode.
Did that change from C99? Not really:
A bit-field shall have a type that is a qualified or unqualified
version of _Bool, signed int, unsigned int, or some other
implementation-defined type.
Even versus C11, which we already require?
Yes, I would say it doesn't change things. Only C23 would improve that:
https://open-std.org/JTC1/SC22/WG14/www/docs/n3030.htm#design-constant.type
by allowing us to define the underlying type.
Ah, ok. Makes sense, I've made that change.
...and in any case we need to make sure to assign
single values in the
enum above: there are no guarantees that FLOW_STATE_ACTIVE is 3
otherwise (except for that static_assert(), but that's not its purpose).
I'm not clear how this comment relates to the one before.
It's unrelated, but:
AFAIK
nothing in here (or the rest of the series) relies on the specific
numeric values of the flow state values (although we do rely on them
being ordered as written in some places).
while we rely on the fact that no value is bigger than 255, I realised
that the standards already guarantee that values start from 0 and every
subsequent constant is defined as one more than the previous one, all
the way from C90 to C11, so this would actually be fine.
Right, I was expecting that behaviour - the way we define
FLOW_NUM_STATES and similar things in a bunch of places relies on
this.
Sorry, I don't know exactly why I thought that
wouldn't be the case, I
was pretty sure of the opposite until I checked.
Ok. I've scattered in some extra static_assert()s in the vicinity, too.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!