[PATCH 0/3] RFC: Allow C11 extensions in the passt/pasta code
As discussed on our recent calls, the C11 standard introduces anonymous structure and union members and static assertions, amongst other things. Both of these could be useful in a few places in passt/pasta to make the code more readable and safer. However, at the moment, the compiler flags we use only allow C99 code. This series allows C11 code, and makes some fairly obvious cleanups by using it. It would be nice to get an opinion on this reasonably quickly, because I have other patches in the works that will look different depending on whether or not they can use C11 features. David Gibson (3): Allow C11 code, not just C99 code Use C11 anonymous members to make poll refs less verbose to use Use static assertion to verify that union epoll_ref is the right size Makefile | 4 ++-- icmp.c | 22 +++++++++++----------- icmp.h | 2 +- passt.c | 8 ++++---- passt.h | 8 ++++++-- tcp.c | 46 +++++++++++++++++++++++----------------------- tcp.h | 2 +- tcp_splice.c | 11 +++++------ udp.c | 50 +++++++++++++++++++++++--------------------------- udp.h | 2 +- util.c | 4 ++-- 11 files changed, 79 insertions(+), 80 deletions(-) -- 2.41.0
C11 has some features that will allow us to make some things a bit cleaner.
Alter the Makefile to tell the compiler to allow us to use C11 code.
Signed-off-by: David Gibson
union epoll_ref has a deeply nested set of structs and unions to let us
subdivide it into the various different fields we want. This means that
referencing elements can involve an awkward long string of intermediate
fields.
Using C11 anonymous structs and unions lets us do this less clumsily.
Signed-off-by: David Gibson
union epoll_ref is used to subdivide the 64-bit data field in struct
epoll_event. Thus it *must* fit within that field or we're likely to get
very subtle and nasty bugs. C11 introduces the notion of static assertions
which we can use to verify this is the case at compile time.
Signed-off-by: David Gibson
On Tue, 1 Aug 2023 13:36:44 +1000
David Gibson
As discussed on our recent calls, the C11 standard introduces anonymous structure and union members and static assertions, amongst other things. Both of these could be useful in a few places in passt/pasta to make the code more readable and safer.
However, at the moment, the compiler flags we use only allow C99 code. This series allows C11 code, and makes some fairly obvious cleanups by using it.
It would be nice to get an opinion on this reasonably quickly, because I have other patches in the works that will look different depending on whether or not they can use C11 features.
...then let me start with this one, as it's straightforward: I think anonymous unions and structures are great. :) The series (especially 2/3) looks good to me, I'll push it in a bit. We also need to check for issues with reasonably older gcc (perhaps those we have in the test/distro tests, at least) and clang versions, unless you already did that. -- Stefano
On Tue, Aug 01, 2023 at 10:15:27AM +0200, Stefano Brivio wrote:
On Tue, 1 Aug 2023 13:36:44 +1000 David Gibson
wrote: As discussed on our recent calls, the C11 standard introduces anonymous structure and union members and static assertions, amongst other things. Both of these could be useful in a few places in passt/pasta to make the code more readable and safer.
However, at the moment, the compiler flags we use only allow C99 code. This series allows C11 code, and makes some fairly obvious cleanups by using it.
It would be nice to get an opinion on this reasonably quickly, because I have other patches in the works that will look different depending on whether or not they can use C11 features.
...then let me start with this one, as it's straightforward: I think anonymous unions and structures are great. :)
The series (especially 2/3) looks good to me, I'll push it in a bit.
We also need to check for issues with reasonably older gcc (perhaps those we have in the test/distro tests, at least) and clang versions, unless you already did that.
So I tried building with this series in some container images of old debian versions. In buster (Debian 10), it builds ok with both gcc 8sh and clang 7ish, although the latter unsurprisingly gives warnings for the gcc specific __attribute__((optimize("-fno-strict-aliasing"))) we have on the siphash functions. In stretch (Debian 9), it builds ok with gcc 6ish, though not with clang-3.8ish. However the latter doesn't appear to be because of the C11 changes - it's complaining about initializers which only list some of the structure fields. In jessie (Debian 8) it doesn't build with gcc 4ish or clang-3.5ish, but again it appears to be the incomplete initializers rather than the C11 changes. In short, it appears that before we hit compilers that won't cope with the C11 changes, we hit compilers that object to incomplete initializers that we're already using. So.. I don't think there's any reason not to apply the C11 changes. -- 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_! http://www.ozlabs.org/~dgibson
On Wed, 2 Aug 2023 14:47:07 +1000
David Gibson
On Tue, Aug 01, 2023 at 10:15:27AM +0200, Stefano Brivio wrote:
On Tue, 1 Aug 2023 13:36:44 +1000 David Gibson
wrote: As discussed on our recent calls, the C11 standard introduces anonymous structure and union members and static assertions, amongst other things. Both of these could be useful in a few places in passt/pasta to make the code more readable and safer.
However, at the moment, the compiler flags we use only allow C99 code. This series allows C11 code, and makes some fairly obvious cleanups by using it.
It would be nice to get an opinion on this reasonably quickly, because I have other patches in the works that will look different depending on whether or not they can use C11 features.
...then let me start with this one, as it's straightforward: I think anonymous unions and structures are great. :)
The series (especially 2/3) looks good to me, I'll push it in a bit.
We also need to check for issues with reasonably older gcc (perhaps those we have in the test/distro tests, at least) and clang versions, unless you already did that.
So I tried building with this series in some container images of old debian versions.
In buster (Debian 10), it builds ok with both gcc 8sh and clang 7ish, although the latter unsurprisingly gives warnings for the gcc specific __attribute__((optimize("-fno-strict-aliasing"))) we have on the siphash functions.
In stretch (Debian 9), it builds ok with gcc 6ish, though not with clang-3.8ish. However the latter doesn't appear to be because of the C11 changes - it's complaining about initializers which only list some of the structure fields.
In jessie (Debian 8) it doesn't build with gcc 4ish or clang-3.5ish, but again it appears to be the incomplete initializers rather than the C11 changes.
Oops.
In short, it appears that before we hit compilers that won't cope with the C11 changes, we hit compilers that object to incomplete initializers that we're already using. So.. I don't think there's any reason not to apply the C11 changes.
Thanks for checking. And yes, all the "new" features used in this series should be supported starting from gcc 4.7 (March 2012) anyway, which seems to be a reasonable target. I'll go ahead and apply this. -- Stefano
On Tue, 1 Aug 2023 13:36:44 +1000
David Gibson
As discussed on our recent calls, the C11 standard introduces anonymous structure and union members and static assertions, amongst other things. Both of these could be useful in a few places in passt/pasta to make the code more readable and safer.
However, at the moment, the compiler flags we use only allow C99 code. This series allows C11 code, and makes some fairly obvious cleanups by using it.
It would be nice to get an opinion on this reasonably quickly, because I have other patches in the works that will look different depending on whether or not they can use C11 features.
David Gibson (3): Allow C11 code, not just C99 code Use C11 anonymous members to make poll refs less verbose to use Use static assertion to verify that union epoll_ref is the right size
Applied. -- Stefano
participants (2)
-
David Gibson
-
Stefano Brivio