From: "Morten Brørup" <mb@smartsharesystems.com>
To: "Stephen Hemminger" <stephen@networkplumber.org>
Cc: "Bruce Richardson" <bruce.richardson@intel.com>, <dev@dpdk.org>,
"Jerin Jacob" <jerinj@marvell.com>
Subject: RE: [PATCH v4] eventdev: ensure 16-byte alignment for events
Date: Mon, 13 Nov 2023 08:58:19 +0100 [thread overview]
Message-ID: <98CBD80474FA8B44BF855DF32C47DC35E9F018@smartserver.smartshare.dk> (raw)
In-Reply-To: <20231112153146.5fdd72b8@hermes.local>
> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> Sent: Monday, 13 November 2023 00.32
>
> On Sun, 12 Nov 2023 09:30:24 +0100
> Morten Brørup <mb@smartsharesystems.com> wrote:
>
> > > > +static_assert(sizeof(struct rte_event) == 16, "Event structure
> size
> > > is not 16-bytes in size");
> > > > +
> > > > static struct rte_eventdev
> rte_event_devices[RTE_EVENT_MAX_DEVS];
> > >
> > > Please don't reinvent RTE_BUILD_BUG_ON().
> > > Instead fix that to be a static_assert()
> >
> > I would say the opposite:
> > With our upgrade to the C11 standard, let's get rid of the
> RTE_BUILD_BUG_ON() workaround for the lack of static_assert() in older
> C standards.
> >
> > Unfortunately, the static_assert(expression) variant without the
> "message" parameter, which would make our RTE_BUILD_BUG_ON() macro
> completely obsolete, requires C23. And I don't see how we can make this
> variant available with C11. So we probably have to wait until DPDK
> requires C23.
> >
> > Until then, let's gradually phase out the DPDK-specific
> RTE_BUILD_BUG_ON() in favor of standard C's static_assert(), and live
> with the inconvenience of having to provide a message parameter for it.
> >
> > Please also note that static_assert() can be used outside code
> blocks, which makes it handy for use in header files.
>
> If you look at my RFC, the message is just as good as the one in this
> code.
> It ends up being stringified version of the expression. Which is more
> exact than the wording used in some other places.
I agree that the output of your RFC is better than that of static_assert(expr, msg).
I'm arguing that RTE_BUILD_BUG_ON() would be considered reinventing the wheel if introduced today, because the C11 standard already offers something similar. So we should prefer that over RTE_BUILD_BUG_ON(), not the other way around.
It's a difference in opinion. Both RTE_BUILD_BUG_ON() and static_assert() are viable; you seem to prefer the first, I prefer the latter.
Both occur in existing DPDK code, and Bruce happened to chose static_assert() for this patch.
The overall question is a choice between three options:
1. prefer using RTE_BUILD_BUG_ON() (using your RFC implementation),
2. phase out RTE_BUILD_BUG_ON() in favor of the C11 standard's static_assert(), or
3. continue using both?
I'm not strongly opposed to either of the three, as long as the community agrees.
next prev parent reply other threads:[~2023-11-13 7:58 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-10-05 11:51 [RFC PATCH] " Bruce Richardson
2023-10-05 12:06 ` Bruce Richardson
2023-10-05 13:11 ` Jerin Jacob
2023-10-05 13:15 ` Bruce Richardson
2023-10-06 7:19 ` Jerin Jacob
2023-10-05 12:12 ` Morten Brørup
2023-10-05 13:02 ` Bruce Richardson
2023-10-06 9:37 ` [PATCH v2] " Bruce Richardson
2023-10-06 9:45 ` [PATCH v3] " Bruce Richardson
2023-10-06 10:13 ` Morten Brørup
2023-10-06 10:16 ` Jerin Jacob
2023-10-06 10:19 ` Bruce Richardson
2023-10-06 10:24 ` Jerin Jacob
2023-10-06 10:27 ` Bruce Richardson
2023-10-06 10:16 ` Bruce Richardson
2023-10-06 10:35 ` Morten Brørup
2023-10-06 10:44 ` Bruce Richardson
2023-10-06 10:29 ` [PATCH v4] " Bruce Richardson
2023-11-12 0:01 ` Stephen Hemminger
2023-11-12 8:30 ` Morten Brørup
2023-11-12 23:31 ` Stephen Hemminger
2023-11-13 7:58 ` Morten Brørup [this message]
2024-01-19 21:05 ` Tyler Retzlaff
2023-10-06 12:15 ` [RFC PATCH] " Mattias Rönnblom
2023-10-06 12:19 ` Bruce Richardson
2023-10-06 12:29 ` Mattias Rönnblom
2024-01-19 22:30 ` Stephen Hemminger
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=98CBD80474FA8B44BF855DF32C47DC35E9F018@smartserver.smartshare.dk \
--to=mb@smartsharesystems.com \
--cc=bruce.richardson@intel.com \
--cc=dev@dpdk.org \
--cc=jerinj@marvell.com \
--cc=stephen@networkplumber.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).