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/pci... , 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
Hi Julius
Sounds good to me. Btw a similar discussion happened on the LKML. https://lore.kernel.org/lkml/CAHk-=whFKYMrF6euVvziW+drw7-yi1pYdf=uccnzJ8k09D... 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/pci... , 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
-
Arthur Heymans -
Julius Werner