Hi Julius

Sounds good to me.
Btw a similar discussion happened on the LKML.
https://lore.kernel.org/lkml/CAHk-=whFKYMrF6euVvziW+drw7-yi1pYdf=uccnzJ8k09DoTXA@mail.gmail.com/ is the upshot,
which is complete agreement with what you said.

Kind regards
Arthur

On Fri, Apr 15, 2022 at 11:30 PM Julius Werner <jwerner@chromium.org> wrote:
Hi,

We occasionally get into discussions in code reviews when code uses a
GCC extension, and a reviewer asks that it be rewritten to be C
standard compliant instead. A recent example was on the use of `void
*` arithmetic in this patch
https://review.coreboot.org/c/coreboot/+/56791/9/src/soc/mediatek/common/pcie.c#109
, and there have been others in the past. I would like to come to a
consensus on this topic here and add some blurb to the coding style
document so we don't have to rehash the same discussion over and over
again.

In my opinion, GCC extensions are absolutely necessary for coreboot
development and there should be no reason to reject them. Inline
assembly is the most obvious example -- without it we would have to
convert a ton of static inline functions that wrap special
instructions into full linker-level functions in a separate assembly
file instead, and eat all the unnecessary function call overhead that
comes with that. Others enable such important features that it would
become much more dangerous and cumbersome to develop without them --
most importantly statement expressions
(https://gcc.gnu.org/onlinedocs/gcc-11.2.0/gcc/Statement-Exprs.html)
which are necessary to write things like double-evaluation safe
macros, expression-assertions like dead_code_t() and simple
convenience shortcuts like wait_us(). And some extensions just offer a
small bit of convenience to reduce boilerplate -- for example, `void
*` arithmetic just tends to be useful to prevent cluttering the code
with a bunch of unnecessary casts to other types that don't add any
additional meaning to the data (e.g. for an unspecified buffer of
opaque data I think `void *` is a much more appropriate type than
`uint8_t *`, even if I want to add a byte offset to it), and I've
never seen a case where I think it would have actually been unclear to
anyone what step width the pointer arithmetic was done at (there's no
reason to assume anything other than bytewise for a `void *`).

If we need some extensions anyway, and coreboot will never be "fully C
standards compliant" (not that that would actually be very useful for
anything in practice), I don't see a reason why we should still avoid
some extensions when we're using others. I think if an (official,
long-term supported) extension exists and it allows us to write better
code than standard C, there should be no reason not to use it. (Note
that for those people who are trying to get coreboot working with
clang, it generally supports all the same extensions as GCC.) I've
written a draft CL to add a section for this to the coding style,
please let me know what you think:
https://review.coreboot.org/c/coreboot/+/63660
_______________________________________________
coreboot mailing list -- coreboot@coreboot.org
To unsubscribe send an email to coreboot-leave@coreboot.org