Change in flashrom[master]: endiantest: Fix #if expression
David Hendricks has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/43598 ) Change subject: endiantest: Fix #if expression ...................................................................... endiantest: Fix #if expression Without this, `gcc -E endiantest.c` can fail and return the incorrect endiannes as well as exiting with non-zero. Here is the actual error shown in the output: endiantest.c:2:31: error: #if with no expression #if __FLASHROM_LITTLE_ENDIAN__ I was able to reproduce this using gcc-6.3.0 and clang-4.0.1, but newer compilers didn't have this issue. Change-Id: Iba2febd861471ec821a494336e800c2564984332 Signed-off-by: David Hendricks <david.hendricks@gmail.com> --- M endiantest.c 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/98/43598/1 diff --git a/endiantest.c b/endiantest.c index de44ec5..9766061 100644 --- a/endiantest.c +++ b/endiantest.c @@ -1,5 +1,5 @@ #include "platform.h" -#if __FLASHROM_LITTLE_ENDIAN__ +#if defined __FLASHROM_LITTLE_ENDIAN__ little #else big -- To view, visit https://review.coreboot.org/c/flashrom/+/43598 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iba2febd861471ec821a494336e800c2564984332 Gerrit-Change-Number: 43598 Gerrit-PatchSet: 1 Gerrit-Owner: David Hendricks <david.hendricks@gmail.com> Gerrit-MessageType: newchange
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/43598 ) Change subject: endiantest: Fix #if expression ...................................................................... Patch Set 1: Code-Review+1 (1 comment) https://review.coreboot.org/c/flashrom/+/43598/1/endiantest.c File endiantest.c: https://review.coreboot.org/c/flashrom/+/43598/1/endiantest.c@2 PS1, Line 2: #if defined __FLASHROM_LITTLE_ENDIAN__ What's the difference vs using #ifdef ? -- To view, visit https://review.coreboot.org/c/flashrom/+/43598 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iba2febd861471ec821a494336e800c2564984332 Gerrit-Change-Number: 43598 Gerrit-PatchSet: 1 Gerrit-Owner: David Hendricks <david.hendricks@gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-Comment-Date: Sun, 19 Jul 2020 21:57:33 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Hello build bot (Jenkins), Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/43598 to look at the new patch set (#2). Change subject: endiantest: Fix #if expression ...................................................................... endiantest: Fix #if expression Without this, `gcc -E endiantest.c` can fail and return the incorrect endiannes as well as exiting with non-zero. Here is the actual error shown in the output: endiantest.c:2:31: error: #if with no expression #if __FLASHROM_LITTLE_ENDIAN__ I was able to reproduce this using gcc-6.3.0 and clang-4.0.1, but newer compilers didn't have this issue. Change-Id: Iba2febd861471ec821a494336e800c2564984332 Signed-off-by: David Hendricks <david.hendricks@gmail.com> --- M endiantest.c 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/98/43598/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/43598 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iba2febd861471ec821a494336e800c2564984332 Gerrit-Change-Number: 43598 Gerrit-PatchSet: 2 Gerrit-Owner: David Hendricks <david.hendricks@gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-MessageType: newpatchset
David Hendricks has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/43598 ) Change subject: endiantest: Fix #if expression ...................................................................... Patch Set 2: (1 comment) https://review.coreboot.org/c/flashrom/+/43598/1/endiantest.c File endiantest.c: https://review.coreboot.org/c/flashrom/+/43598/1/endiantest.c@2 PS1, Line 2: #if defined __FLASHROM_LITTLE_ENDIAN__
What's the difference vs using #ifdef ? No difference according to https://gcc.gnu.org/onlinedocs/gcc-8.4.0/cpp/Defined.html
I used "#if defined" since that's what we use in the other related files (os.h, archtest.c, platform.h). Actually, I just realized that the other files put parens around the macro name, so I've done that in PS2. Now if only we were consistent with spacing... -- To view, visit https://review.coreboot.org/c/flashrom/+/43598 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iba2febd861471ec821a494336e800c2564984332 Gerrit-Change-Number: 43598 Gerrit-PatchSet: 2 Gerrit-Owner: David Hendricks <david.hendricks@gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-Comment-Date: Sun, 19 Jul 2020 23:07:09 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus@gmail.com> Gerrit-MessageType: comment
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/43598 ) Change subject: endiantest: Fix #if expression ...................................................................... Patch Set 2: Code-Review+2 (1 comment) https://review.coreboot.org/c/flashrom/+/43598/1/endiantest.c File endiantest.c: https://review.coreboot.org/c/flashrom/+/43598/1/endiantest.c@2 PS1, Line 2: #if defined __FLASHROM_LITTLE_ENDIAN__
No difference according to https://gcc.gnu.org/onlinedocs/gcc-8.4.0/cpp/Defined.html […] OK, good enough.
-- To view, visit https://review.coreboot.org/c/flashrom/+/43598 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iba2febd861471ec821a494336e800c2564984332 Gerrit-Change-Number: 43598 Gerrit-PatchSet: 2 Gerrit-Owner: David Hendricks <david.hendricks@gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-Comment-Date: Sun, 19 Jul 2020 23:32:21 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: David Hendricks <david.hendricks@gmail.com> Comment-In-Reply-To: Angel Pons <th3fanbus@gmail.com> Gerrit-MessageType: comment
David Hendricks has submitted this change. ( https://review.coreboot.org/c/flashrom/+/43598 ) Change subject: endiantest: Fix #if expression ...................................................................... endiantest: Fix #if expression Without this, `gcc -E endiantest.c` can fail and return the incorrect endiannes as well as exiting with non-zero. Here is the actual error shown in the output: endiantest.c:2:31: error: #if with no expression #if __FLASHROM_LITTLE_ENDIAN__ I was able to reproduce this using gcc-6.3.0 and clang-4.0.1, but newer compilers didn't have this issue. Change-Id: Iba2febd861471ec821a494336e800c2564984332 Signed-off-by: David Hendricks <david.hendricks@gmail.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/43598 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Angel Pons <th3fanbus@gmail.com> --- M endiantest.c 1 file changed, 1 insertion(+), 1 deletion(-) Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved diff --git a/endiantest.c b/endiantest.c index de44ec5..a73b908 100644 --- a/endiantest.c +++ b/endiantest.c @@ -1,5 +1,5 @@ #include "platform.h" -#if __FLASHROM_LITTLE_ENDIAN__ +#if defined(__FLASHROM_LITTLE_ENDIAN__) little #else big -- To view, visit https://review.coreboot.org/c/flashrom/+/43598 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iba2febd861471ec821a494336e800c2564984332 Gerrit-Change-Number: 43598 Gerrit-PatchSet: 3 Gerrit-Owner: David Hendricks <david.hendricks@gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: David Hendricks <david.hendricks@gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-MessageType: merged
participants (2)
-
Angel Pons (Code Review) -
David Hendricks (Code Review)