Change in ...coreboot[master]: src/security/vboot/misc.h Fix clang fail to compile

List overview All Threads
Download

newer

older

Change in ...coreboot[master]:...

Alan Green (Code Review)

15 May 2019 15 May '19

4:19 a.m.

Alan Green has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32798

Change subject: src/security/vboot/misc.h Fix clang fail to compile ......................................................................

src/security/vboot/misc.h Fix clang fail to compile

Clang does not recognize dead_code() as termination of execution. It gives this message:

error: control reaches end of non-void function [-Werror,-Wreturn-type]

The error was being encountered even without VBOOT enabled, because the misc.h file is included from many places, including coreboot_table.c.

This change:

(a) provides dummy return values from the dead code paths to keep clang happy. (b) moves the final case of if-else-if statements out of else clause to keep the linter happy.

Signed-off-by: Alan Green avg@google.com Change-Id: I19e0c9ba36f0ea5110480521a5f9afa0112a3137 --- M src/security/vboot/misc.h 1 file changed, 6 insertions(+), 6 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/98/32798/1

diff --git a/src/security/vboot/misc.h b/src/security/vboot/misc.h index 23159c8..03bf663 100644 --- a/src/security/vboot/misc.h +++ b/src/security/vboot/misc.h @@ -78,7 +78,6 @@ * code should be called. They are implemented inline for better compile-time * code elimination. */ - static inline int verification_should_run(void) { if (CONFIG(VBOOT_SEPARATE_VERSTAGE)) @@ -87,8 +86,9 @@ return ENV_ROMSTAGE; else if (CONFIG(VBOOT_STARTS_IN_BOOTBLOCK)) return ENV_BOOTBLOCK; - else - dead_code(); + + dead_code(); + return 0; }

static inline int verstage_should_load(void) @@ -120,10 +120,10 @@ #else return 1; #endif - } else { - dead_code(); } -}

+ dead_code(); + return 0; +}

#endif /* __VBOOT_MISC_H__ */

Attachments:

attachment.htm (text/html — 3.9 KB)

Show replies by date

Alan Green (Code Review)

15 May 15 May

5:17 a.m.

Alan Green has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32798 )

Change subject: src/security/vboot/misc.h Fix clang fail to compile ......................................................................

Patch Set 1:

I did consider guarding the three inline functions with #if CONFIG(VBOOT) but it seems unnecessary.

-- To view, visit https://review.coreboot.org/c/coreboot/+/32798 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I19e0c9ba36f0ea5110480521a5f9afa0112a3137 Gerrit-Change-Number: 32798 Gerrit-PatchSet: 1 Gerrit-Owner: Alan Green avg@google.com Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Alan Green avg@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Julius Werner jwerner@chromium.org Gerrit-Comment-Date: Wed, 15 May 2019 03:17:49 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

Alan Green (Code Review)

5:19 a.m.

Hello Aaron Durbin, build bot (Jenkins),

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/32798

to look at the new patch set (#2).

Change subject: src/security/vboot/misc.h Fix clang fail to compile ......................................................................

src/security/vboot/misc.h Fix clang fail to compile

Clang does not recognize dead_code() as termination of execution. It gives this message:

error: control reaches end of non-void function [-Werror,-Wreturn-type]

The error was being encountered even without VBOOT enabled, because the misc.h file is included from many places, including coreboot_table.c.

This change:

(a) provides dummy return values from the dead code paths to keep clang happy. (b) moves the final case of if-else-if statements out of else clause to keep the linter happy.

Signed-off-by: Alan Green avg@google.com Change-Id: I19e0c9ba36f0ea5110480521a5f9afa0112a3137 --- M src/security/vboot/misc.h 1 file changed, 6 insertions(+), 5 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/98/32798/2

Aaron Durbin (Code Review)

5:36 a.m.

Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32798 )

Change subject: src/security/vboot/misc.h Fix clang fail to compile ......................................................................

Patch Set 2: Code-Review+2

-- To view, visit https://review.coreboot.org/c/coreboot/+/32798 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I19e0c9ba36f0ea5110480521a5f9afa0112a3137 Gerrit-Change-Number: 32798 Gerrit-PatchSet: 2 Gerrit-Owner: Alan Green avg@google.com Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Alan Green avg@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Julius Werner jwerner@chromium.org Gerrit-Comment-Date: Wed, 15 May 2019 03:36:39 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Furquan Shaikh (Code Review)

6 a.m.

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32798 )

Change subject: src/security/vboot/misc.h Fix clang fail to compile ......................................................................

Patch Set 2: Code-Review+2

-- To view, visit https://review.coreboot.org/c/coreboot/+/32798 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I19e0c9ba36f0ea5110480521a5f9afa0112a3137 Gerrit-Change-Number: 32798 Gerrit-PatchSet: 2 Gerrit-Owner: Alan Green avg@google.com Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Alan Green avg@google.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Julius Werner jwerner@chromium.org Gerrit-Comment-Date: Wed, 15 May 2019 04:00:11 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Patrick Georgi (Code Review)

7:53 p.m.

Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32798 )

Change subject: src/security/vboot/misc.h Fix clang fail to compile ......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/#/c/32798/2/src/security/vboot/misc.h File src/security/vboot/misc.h:

https://review.coreboot.org/#/c/32798/2/src/security/vboot/misc.h@92 PS2, Line 92: return does it work to mark dead_code_assertion_failed_at_line_##line() as __attribute__((noreturn)) in assert.h? that way we don't have to rely on sprinkling spurious return statements across the tree.

-- To view, visit https://review.coreboot.org/c/coreboot/+/32798 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I19e0c9ba36f0ea5110480521a5f9afa0112a3137 Gerrit-Change-Number: 32798 Gerrit-PatchSet: 2 Gerrit-Owner: Alan Green avg@google.com Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Alan Green avg@google.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Julius Werner jwerner@chromium.org Gerrit-CC: Patrick Georgi pgeorgi@google.com Gerrit-Comment-Date: Wed, 15 May 2019 17:53:03 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

Julius Werner (Code Review)

10:34 p.m.

Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32798 )

Change subject: src/security/vboot/misc.h Fix clang fail to compile ......................................................................

Patch Set 2:

(1 comment)

I'm surprised that GCC even compiles that, now that I think about it. I guess it must be doing the dead code elimination before type checking.

https://review.coreboot.org/#/c/32798/2/src/security/vboot/misc.h File src/security/vboot/misc.h:

https://review.coreboot.org/#/c/32798/2/src/security/vboot/misc.h@92 PS2, Line 92: return

...

does it work to mark dead_code_assertion_failed_at_line_##line() as __attribute__((noreturn)) in ass […]

Agreed, this should work. If not, you could also use

return dead_code_t(int);

-- To view, visit https://review.coreboot.org/c/coreboot/+/32798 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I19e0c9ba36f0ea5110480521a5f9afa0112a3137 Gerrit-Change-Number: 32798 Gerrit-PatchSet: 2 Gerrit-Owner: Alan Green avg@google.com Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Alan Green avg@google.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Julius Werner jwerner@chromium.org Gerrit-CC: Patrick Georgi pgeorgi@google.com Gerrit-Comment-Date: Wed, 15 May 2019 20:34:20 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Patrick Georgi pgeorgi@google.com Gerrit-MessageType: comment

Patrick Georgi (Code Review)

17 May 17 May

9:17 a.m.

Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32798 )

Change subject: src/security/vboot/misc.h Fix clang fail to compile ......................................................................

Patch Set 2:

Is this still necessary in light of CB:32833?

-- To view, visit https://review.coreboot.org/c/coreboot/+/32798 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I19e0c9ba36f0ea5110480521a5f9afa0112a3137 Gerrit-Change-Number: 32798 Gerrit-PatchSet: 2 Gerrit-Owner: Alan Green avg@google.com Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Alan Green avg@google.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Julius Werner jwerner@chromium.org Gerrit-CC: Patrick Georgi pgeorgi@google.com Gerrit-Comment-Date: Fri, 17 May 2019 07:17:10 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

Alan Green (Code Review)

19 May 19 May

10:04 p.m.

Alan Green has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32798 )

Change subject: src/security/vboot/misc.h Fix clang fail to compile ......................................................................

Patch Set 2:

(1 comment)

...

Patch Set 2:

Is this still necessary in light of CB:32833?

No, it is not.

https://review.coreboot.org/#/c/32798/2/src/security/vboot/misc.h File src/security/vboot/misc.h:

https://review.coreboot.org/#/c/32798/2/src/security/vboot/misc.h@92 PS2, Line 92: return

...

Agreed, this should work. If not, you could also use […]

It does work.

See new patch at https://review.coreboot.org/c/coreboot/+/32833

-- To view, visit https://review.coreboot.org/c/coreboot/+/32798 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I19e0c9ba36f0ea5110480521a5f9afa0112a3137 Gerrit-Change-Number: 32798 Gerrit-PatchSet: 2 Gerrit-Owner: Alan Green avg@google.com Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Alan Green avg@google.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Julius Werner jwerner@chromium.org Gerrit-CC: Patrick Georgi pgeorgi@google.com Gerrit-Comment-Date: Sun, 19 May 2019 20:04:41 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Julius Werner jwerner@chromium.org Comment-In-Reply-To: Patrick Georgi pgeorgi@google.com Gerrit-MessageType: comment

Alan Green (Code Review)

10:07 p.m.

Alan Green has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/32798 )

Change subject: src/security/vboot/misc.h Fix clang fail to compile ......................................................................

Abandoned

Superceded by CB:32833

-- To view, visit https://review.coreboot.org/c/coreboot/+/32798 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I19e0c9ba36f0ea5110480521a5f9afa0112a3137 Gerrit-Change-Number: 32798 Gerrit-PatchSet: 2 Gerrit-Owner: Alan Green avg@google.com Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Alan Green avg@google.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Julius Werner jwerner@chromium.org Gerrit-CC: Patrick Georgi pgeorgi@google.com Gerrit-MessageType: abandon

2177

days inactive

2181

days old

coreboot-gerrit@coreboot.org

9 comments

5 participants

tags (0)

participants (5)

Aaron Durbin (Code Review)
Alan Green (Code Review)
Furquan Shaikh (Code Review)
Julius Werner (Code Review)
Patrick Georgi (Code Review)