ron minnich has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37202 )
Change subject: libpayload: Make pci and endian handling -Wconversion safe ......................................................................
Patch Set 1:
(3 comments)
https://review.coreboot.org/c/coreboot/+/37202/1/payloads/libpayload/include... File payloads/libpayload/include/endian.h:
https://review.coreboot.org/c/coreboot/+/37202/1/payloads/libpayload/include... PS1, Line 39: return (uint16_t)((in & 0xFF) << 8) | ((in & 0xFF00) >> 8); every time I see stuff like this I refer to the byte order fallacy, is there certainty that we need this. https://commandcenter.blogspot.com/2012/04/byte-order-fallacy.html
https://review.coreboot.org/c/coreboot/+/37202/1/payloads/libpayload/include... File payloads/libpayload/include/pci.h:
https://review.coreboot.org/c/coreboot/+/37202/1/payloads/libpayload/include... PS1, Line 94: #define PCI_DEV(_bus, _dev, _fn) (0x80000000 | \ is there any reason for this to be a macro? I know that was the style when we started but ... why not just make it an inline at least and buy a little type safety? I think we might start adopting the rule that if it has parens, we make it a function.
https://review.coreboot.org/c/coreboot/+/37202/1/payloads/libpayload/libpci/... File payloads/libpayload/libpci/libpci.c:
https://review.coreboot.org/c/coreboot/+/37202/1/payloads/libpayload/libpci/... PS1, Line 43: return pci_read_config8(libpci_to_lb(dev), (uint16_t)pos); is it worth just forcing the parameter to be a uint?