Change in ...coreboot[master]: assert.h: Undefine ASSERT macro in case vendorcode headers set it
Hello Aaron Durbin, Frans Hendriks, Philipp Deppenwiese,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/32715
to review the following change.
Change subject: assert.h: Undefine ASSERT macro in case vendorcode headers set it ......................................................................
assert.h: Undefine ASSERT macro in case vendorcode headers set it
Some edk2 vendorcode headers define an ASSERT macro. They're guarded with an #ifndef ASSERT, but if coreboot's assert.h gets included after that header, we still have a problem. Add code to assert.h to undefine any rogue definitions that may have already been set by vendorcode headers.
This is ugly and should only be a stopgap... it would be nice if someone maintaining those vendorcode parts could eventually replace it with a better solution. One option would be to use a "guard header" for every vendorcode header we want to pull into normal coreboot code which would chain-include the vendorcode header and then undefine anything that clashes with coreboot again.
Change-Id: Ibf8dc8b2365821e401ce69705df20aa7540aefb2 Signed-off-by: Julius Werner jwerner@chromium.org --- M src/include/assert.h 1 file changed, 6 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/15/32715/1
diff --git a/src/include/assert.h b/src/include/assert.h index 60366352..4575a29 100644 --- a/src/include/assert.h +++ b/src/include/assert.h @@ -19,6 +19,12 @@ #include <arch/hlt.h> #include <console/console.h>
+/* TODO: Fix vendorcode headers to not define macros coreboot uses or to be more + properly isolated. */ +#ifdef ASSERT +#undef ASSERT +#endif + /* GCC and CAR versions */ #define ASSERT(x) { \ if (!(x)) { \
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32715 )
Change subject: assert.h: Undefine ASSERT macro in case vendorcode headers set it ......................................................................
Patch Set 1: Code-Review+2
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32715 )
Change subject: assert.h: Undefine ASSERT macro in case vendorcode headers set it ......................................................................
Patch Set 1: Code-Review+2
Julius Werner has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/32715 )
Change subject: assert.h: Undefine ASSERT macro in case vendorcode headers set it ......................................................................
assert.h: Undefine ASSERT macro in case vendorcode headers set it
Some edk2 vendorcode headers define an ASSERT macro. They're guarded with an #ifndef ASSERT, but if coreboot's assert.h gets included after that header, we still have a problem. Add code to assert.h to undefine any rogue definitions that may have already been set by vendorcode headers.
This is ugly and should only be a stopgap... it would be nice if someone maintaining those vendorcode parts could eventually replace it with a better solution. One option would be to use a "guard header" for every vendorcode header we want to pull into normal coreboot code which would chain-include the vendorcode header and then undefine anything that clashes with coreboot again.
Change-Id: Ibf8dc8b2365821e401ce69705df20aa7540aefb2 Signed-off-by: Julius Werner jwerner@chromium.org Reviewed-on: https://review.coreboot.org/c/coreboot/+/32715 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Aaron Durbin adurbin@chromium.org Reviewed-by: Furquan Shaikh furquan@google.com --- M src/include/assert.h 1 file changed, 6 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Aaron Durbin: Looks good to me, approved Furquan Shaikh: Looks good to me, approved
diff --git a/src/include/assert.h b/src/include/assert.h index 60366352..4575a29 100644 --- a/src/include/assert.h +++ b/src/include/assert.h @@ -19,6 +19,12 @@ #include <arch/hlt.h> #include <console/console.h>
+/* TODO: Fix vendorcode headers to not define macros coreboot uses or to be more + properly isolated. */ +#ifdef ASSERT +#undef ASSERT +#endif + /* GCC and CAR versions */ #define ASSERT(x) { \ if (!(x)) { \
-
Aaron Durbin (Code Review) -
Furquan Shaikh (Code Review)
-
Julius Werner (Code Review)