Attention is currently required from: Nico Huber, Thomas Heijligen.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/70139 )
Change subject: libpayload: Outsource delay function into own header ......................................................................
Patch Set 8:
(1 comment)
File payloads/libpayload/include/libpayload.h:
https://review.coreboot.org/c/coreboot/+/70139/comment/b26dec48_2be9f24f PS2, Line 71: libpayload
Julius, could you have another look?
(Sorry for the delay, have been super swamped for a while.)
So, there are a couple of solutions here. One is, like you suggest, to move all libpayload headers into a subdirectory namespace. But that's a big change that will cause disruption to all payloads, and I also think it's a bit inconsistent because traditionally C doesn't do that (e.g. it's <stdio.h>, not <std/stdio.h>) and it's instead up to the programs using libraries to not trample on the library's headers. Many other C libraries follow that principle (e.g. if you're using libpng, you include <png.h> not <libpng/png.h>), so it doesn't seem "wrong" that libpayload does the same.
The other is that the app (in this case flashrom) needs to change its header names to avoid any used by libpayload. That's pretty much the standard solution that C userspace utilities using libraries normally use. That's what you'd do if any other library dependency of flashrom was clashing with your headers, too.
The third option (which I tried to allude to earlier but which is not as automatic as I thought) is to make libpayload use "system includes" (angled bracket includes) and the payload (or flashrom in your case) use "local includes" (double quote includes). That way, the payload file could say `include "fmap.h"` to include flashrom's local fmap.h and libpayload could say `include <fmap.h>` to include libpayload's version and there would be no ambiguity.
The problem is that the default `-I` flag always adds directories to be scanned for both kinds of includes. In order to make this work as described, you'd have to make flashrom add its include directory with `-iquote` instead of `-I`.
-
Julius Werner (Code Review)