Change in coreboot[master]: option.h: Add strongly-typed `get_option` wrappers

List overview All Threads
Download

newer

older

Change in coreboot[master]:...

Angel Pons (Code Review)

2 Nov 2020 2 Nov '20

9:51 p.m.

Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/47107 )

Change subject: option.h: Add strongly-typed `get_option` wrappers ......................................................................

option.h: Add strongly-typed `get_option` wrappers

Most users of `get_option` have a value to fall back to in case there's an error. Introduce `read_option_X` wrappers that return the CMOS value on success and a specified fallback value on failure. Furthermore, they are strongly-typed to ease dropping the highly-unsafe `get_option` API.

Change-Id: I6bbc51135216f34518cfd05c3dc90fb68404c1cc Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/include/option.h 1 file changed, 18 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/07/47107/1

diff --git a/src/include/option.h b/src/include/option.h index 6b4e6a9..77f7056 100644 --- a/src/include/option.h +++ b/src/include/option.h @@ -26,4 +26,22 @@ return CB_CMOS_OTABLE_DISABLED; }

+/* + * Convenient, strongly-typed helpers. To avoid redundancy, generate + * them using a macro. The odd name is intentional, to ease grepping. + */ +#define __MAKE_read_option_(type) \ + static inline type read_option_ ## type (const char *name, const type fallback) \ + { \ + type value; \ + return get_option(&value, name) == CB_SUCCESS ? value : fallback; \ + } \ + +__MAKE_read_option_(u8) + +__MAKE_read_option_(int) + +/* Clean up */ +#undef __MAKE_read_option_ + #endif /* _OPTION_H_ */

Attachments:

attachment.htm (text/html — 3.7 KB)

Show replies by date

build bot (Jenkins) (Code Review)

2 Nov 2 Nov

9:52 p.m.

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47107 )

Change subject: option.h: Add strongly-typed `get_option` wrappers ......................................................................

Patch Set 1:

(2 comments)

https://review.coreboot.org/c/coreboot/+/47107/1/src/include/option.h File src/include/option.h:

https://review.coreboot.org/c/coreboot/+/47107/1/src/include/option.h@33 PS1, Line 33: #define __MAKE_read_option_(type) \ Macros with complex values should be enclosed in parentheses

https://review.coreboot.org/c/coreboot/+/47107/1/src/include/option.h@34 PS1, Line 34: static inline type read_option_ ## type (const char *name, const type fallback) \ space prohibited between function name and open parenthesis '('

build bot (Jenkins) (Code Review)

10:19 p.m.

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47107 )

Change subject: option.h: Add strongly-typed `get_option` wrappers ......................................................................

Patch Set 2:

(2 comments)

https://review.coreboot.org/c/coreboot/+/47107/2/src/include/option.h File src/include/option.h:

https://review.coreboot.org/c/coreboot/+/47107/2/src/include/option.h@33 PS2, Line 33: #define __MAKE_read_option_(type) \ Macros with complex values should be enclosed in parentheses

https://review.coreboot.org/c/coreboot/+/47107/2/src/include/option.h@34 PS2, Line 34: static inline type read_option_ ## type (const char *name, const type fallback) \ space prohibited between function name and open parenthesis '('

Nico Huber (Code Review)

2 Dec 2 Dec

10:20 p.m.

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47107 )

Change subject: option.h: Add strongly-typed `get_option` wrappers ......................................................................

Patch Set 2:

(2 comments)

https://review.coreboot.org/c/coreboot/+/47107/2/src/include/option.h File src/include/option.h:

https://review.coreboot.org/c/coreboot/+/47107/2/src/include/option.h@34 PS2, Line 34: read_option_ I remember I mentioned `read_option()`. But in this context I would leave it to `get_option`. Also, how about moving the type in front of `option`? e.g. `get_int_option()` is easier to read imho. What do you think?

https://review.coreboot.org/c/coreboot/+/47107/2/src/include/option.h@36 PS2, Line 36: type value; \ For the moment it seems a good idea to initialize it to 0. At least on little-endian that gives us a sane value if the option is actually shorter. e.g. somebody calls `get_int_option("gfx_uma", 4)` but `gfx_uma` is only 4 bits in CMOS; get_option() would leave the more significant bytes uninitialized.

-- To view, visit https://review.coreboot.org/c/coreboot/+/47107 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I6bbc51135216f34518cfd05c3dc90fb68404c1cc Gerrit-Change-Number: 47107 Gerrit-PatchSet: 2 Gerrit-Owner: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Wed, 02 Dec 2020 22:20:54 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

Angel Pons (Code Review)

5 Dec 5 Dec

6:24 p.m.

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47107 )

Change subject: option.h: Add strongly-typed `get_option` wrappers ......................................................................

Patch Set 2:

(2 comments)

https://review.coreboot.org/c/coreboot/+/47107/2/src/include/option.h File src/include/option.h:

https://review.coreboot.org/c/coreboot/+/47107/2/src/include/option.h@34 PS2, Line 34: read_option_

...

I remember I mentioned `read_option()`. But in this context I would leave […]

I purposefully kept "read_option" unbroken in the function names to allow grepping for these. I can rename them, but I forgot what I wanted the greppability for.

https://review.coreboot.org/c/coreboot/+/47107/2/src/include/option.h@36 PS2, Line 36: type value; \

...

For the moment it seems a good idea to initialize it to 0. At least on […]

I can do it.

-- To view, visit https://review.coreboot.org/c/coreboot/+/47107 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I6bbc51135216f34518cfd05c3dc90fb68404c1cc Gerrit-Change-Number: 47107 Gerrit-PatchSet: 2 Gerrit-Owner: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Sat, 05 Dec 2020 18:24:42 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber nico.h@gmx.de Gerrit-MessageType: comment

Angel Pons (Code Review)

8 Feb 8 Feb

2:56 p.m.

Hello build bot (Jenkins), Nico Huber, Patrick Georgi, Arthur Heymans, Patrick Rudolph,

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

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

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

Change subject: option.h: Add strongly-typed `get_option` wrappers ......................................................................

option.h: Add strongly-typed `get_option` wrappers

Most users of `get_option` have a value to fall back to in case there's an error. Introduce `get_X_option` wrappers which return the CMOS value on success and a specified fallback value on failure. Furthermore, they are strongly-typed to ease dropping the highly-unsafe `get_option` API.

Change-Id: I6bbc51135216f34518cfd05c3dc90fb68404c1cc Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/include/option.h 1 file changed, 17 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/07/47107/3

Angel Pons (Code Review)

2:56 p.m.

Attention is currently required from: Nico Huber. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47107 )

Change subject: option.h: Add strongly-typed `get_option` wrappers ......................................................................

Patch Set 2:

(2 comments)

File src/include/option.h:

https://review.coreboot.org/c/coreboot/+/47107/comment/578d7735_ed23e030 PS2, Line 34: read_option_

...

I purposefully kept "read_option" unbroken in the function names to allow grepping for these. […]

Done

https://review.coreboot.org/c/coreboot/+/47107/comment/f20b1a26_3872874c PS2, Line 36: type value; \

...

I can do it.

Done

-- To view, visit https://review.coreboot.org/c/coreboot/+/47107 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I6bbc51135216f34518cfd05c3dc90fb68404c1cc Gerrit-Change-Number: 47107 Gerrit-PatchSet: 2 Gerrit-Owner: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Attention: Nico Huber nico.h@gmx.de Gerrit-Comment-Date: Mon, 08 Feb 2021 14:56:55 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber nico.h@gmx.de Comment-In-Reply-To: Angel Pons th3fanbus@gmail.com Gerrit-MessageType: comment

build bot (Jenkins) (Code Review)

2:57 p.m.

Attention is currently required from: Nico Huber. build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47107 )

Change subject: option.h: Add strongly-typed `get_option` wrappers ......................................................................

Patch Set 3:

(1 comment)

File src/include/option.h:

https://review.coreboot.org/c/coreboot/+/47107/comment/1210ea69_c5435fac PS3, Line 32: #define __MAKE_get_option_(type) \ Macros with complex values should be enclosed in parentheses

-- To view, visit https://review.coreboot.org/c/coreboot/+/47107 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I6bbc51135216f34518cfd05c3dc90fb68404c1cc Gerrit-Change-Number: 47107 Gerrit-PatchSet: 3 Gerrit-Owner: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Attention: Nico Huber nico.h@gmx.de Gerrit-Comment-Date: Mon, 08 Feb 2021 14:57:03 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

build bot (Jenkins) (Code Review)

3 Mar 3 Mar

4:23 p.m.

Attention is currently required from: Nico Huber. build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47107 )

Change subject: option.h: Add strongly-typed `get_option` wrappers ......................................................................

Patch Set 4:

(1 comment)

File src/include/option.h:

https://review.coreboot.org/c/coreboot/+/47107/comment/8a3e6877_956a3ef3 PS4, Line 32: #define __MAKE_get_option_(type) \ Macros with complex values should be enclosed in parentheses

-- To view, visit https://review.coreboot.org/c/coreboot/+/47107 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I6bbc51135216f34518cfd05c3dc90fb68404c1cc Gerrit-Change-Number: 47107 Gerrit-PatchSet: 4 Gerrit-Owner: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Attention: Nico Huber nico.h@gmx.de Gerrit-Comment-Date: Wed, 03 Mar 2021 16:23:24 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

Angel Pons (Code Review)

19 Apr 19 Apr

1:52 p.m.

New subject: Change in coreboot[master]: option.h: Introduce {get,set}_int_option() functions

Attention is currently required from: Nico Huber. Hello build bot (Jenkins), Nico Huber, Patrick Georgi, Arthur Heymans, Patrick Rudolph,

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

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

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

Change subject: option.h: Introduce {get,set}_int_option() functions ......................................................................

option.h: Introduce {get,set}_int_option() functions

The {get,set}_option() functions are not type-safe: they take a pointer to void, without an associated length/size. Moreover, cmos_get_option() does not always fully initialise the destination value (it has no means to know how large it is), which can cause issues if the caller does not initialise the value beforehand.

The idea behind this patch series is to replace the current type-unsafe API with a type-safe equivalent, and ultimately decouple the option API from CMOS. This would allow using different storage mechanisms with the same option system, maximising flexibility.

Most, if not all users of get_option() have a value to fall back to, in case the option could not be read. Thus, get_int_option() takes a value to fall back to, which avoids repeating the same logic on call-sites.

These new functions will be put to use in subsequent commits.

Change-Id: I6bbc51135216f34518cfd05c3dc90fb68404c1cc Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/include/option.h 1 file changed, 11 insertions(+), 2 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/07/47107/5

-- To view, visit https://review.coreboot.org/c/coreboot/+/47107 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I6bbc51135216f34518cfd05c3dc90fb68404c1cc Gerrit-Change-Number: 47107 Gerrit-PatchSet: 5 Gerrit-Owner: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@mailbox.org Gerrit-Attention: Nico Huber nico.h@gmx.de Gerrit-MessageType: newpatchset

Angel Pons (Code Review)

2:10 p.m.

New subject: Change in coreboot[master]: option.h: Introduce {get,set}_int_option() functions

Attention is currently required from: Nico Huber, Angel Pons. Hello build bot (Jenkins), Nico Huber, Patrick Georgi, Arthur Heymans, Patrick Rudolph,

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

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

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

Change subject: option.h: Introduce {get,set}_int_option() functions ......................................................................

option.h: Introduce {get,set}_int_option() functions

These new functions will be put to use in subsequent commits.

Change-Id: I6bbc51135216f34518cfd05c3dc90fb68404c1cc Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/include/option.h 1 file changed, 11 insertions(+), 2 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/07/47107/6

-- To view, visit https://review.coreboot.org/c/coreboot/+/47107 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I6bbc51135216f34518cfd05c3dc90fb68404c1cc Gerrit-Change-Number: 47107 Gerrit-PatchSet: 6 Gerrit-Owner: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@mailbox.org Gerrit-Attention: Nico Huber nico.h@gmx.de Gerrit-Attention: Angel Pons th3fanbus@gmail.com Gerrit-MessageType: newpatchset

Patrick Rudolph (Code Review)

5:04 p.m.

New subject: Change in coreboot[master]: option.h: Introduce {get,set}_int_option() functions

Attention is currently required from: Nico Huber, Angel Pons. Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47107 )

Change subject: option.h: Introduce {get,set}_int_option() functions ......................................................................

Patch Set 6: Code-Review+1

(2 comments)

File src/include/option.h:

https://review.coreboot.org/c/coreboot/+/47107/comment/976b6632_6218382f PS6, Line 29: set_option should this print an error on failure to set the CMOS option? Or use an assertion?

https://review.coreboot.org/c/coreboot/+/47107/comment/18c6f00f_be631ec8 PS6, Line 35: return get_option(&value, name) == CB_SUCCESS ? value : fallback; should this print an error on failure to set the CMOS option? Or use an assertion?

-- To view, visit https://review.coreboot.org/c/coreboot/+/47107 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I6bbc51135216f34518cfd05c3dc90fb68404c1cc Gerrit-Change-Number: 47107 Gerrit-PatchSet: 6 Gerrit-Owner: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@mailbox.org Gerrit-Attention: Nico Huber nico.h@gmx.de Gerrit-Attention: Angel Pons th3fanbus@gmail.com Gerrit-Comment-Date: Mon, 19 Apr 2021 17:04:03 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Angel Pons (Code Review)

5:16 p.m.

New subject: Change in coreboot[master]: option.h: Introduce {get,set}_int_option() functions

Attention is currently required from: Nico Huber, Patrick Rudolph. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47107 )

Change subject: option.h: Introduce {get,set}_int_option() functions ......................................................................

Patch Set 6:

(2 comments)

File src/include/option.h:

https://review.coreboot.org/c/coreboot/+/47107/comment/c15565ac_18a4f183 PS6, Line 6: #include <types.h> Eh, enum cb_err

https://review.coreboot.org/c/coreboot/+/47107/comment/cbe32195_6b236ff1 PS6, Line 29: set_option

...

should this print an error on failure to set the CMOS option? Or use an assertion?

No, why? If `cmos_set_option` runs into some error, it can print a more accurate error message. Plus, if CMOS is not enabled, errors would be expected.

About using an assertion: assertions are supposed to be fatal. While errors in the option code may indicate something more serious is going on, it's not enough of a reason to halt.

-- To view, visit https://review.coreboot.org/c/coreboot/+/47107 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I6bbc51135216f34518cfd05c3dc90fb68404c1cc Gerrit-Change-Number: 47107 Gerrit-PatchSet: 6 Gerrit-Owner: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@mailbox.org Gerrit-Attention: Nico Huber nico.h@gmx.de Gerrit-Attention: Patrick Rudolph siro@das-labor.org Gerrit-Comment-Date: Mon, 19 Apr 2021 17:16:16 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Patrick Rudolph siro@das-labor.org Gerrit-MessageType: comment

Angel Pons (Code Review)

5:17 p.m.

New subject: Change in coreboot[master]: option.h: Introduce {get,set}_int_option() functions

Attention is currently required from: Nico Huber, Patrick Rudolph. Hello build bot (Jenkins), Nico Huber, Patrick Georgi, Arthur Heymans, Patrick Rudolph,

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

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

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

Change subject: option.h: Introduce {get,set}_int_option() functions ......................................................................

option.h: Introduce {get,set}_int_option() functions

These new functions will be put to use in subsequent commits.

Change-Id: I6bbc51135216f34518cfd05c3dc90fb68404c1cc Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/include/option.h 1 file changed, 11 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/07/47107/7

-- To view, visit https://review.coreboot.org/c/coreboot/+/47107 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I6bbc51135216f34518cfd05c3dc90fb68404c1cc Gerrit-Change-Number: 47107 Gerrit-PatchSet: 7 Gerrit-Owner: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@mailbox.org Gerrit-Attention: Nico Huber nico.h@gmx.de Gerrit-Attention: Patrick Rudolph siro@das-labor.org Gerrit-MessageType: newpatchset

Angel Pons (Code Review)

5:17 p.m.

New subject: Change in coreboot[master]: option.h: Introduce {get,set}_int_option() functions

Attention is currently required from: Nico Huber, Patrick Rudolph. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47107 )

Change subject: option.h: Introduce {get,set}_int_option() functions ......................................................................

Patch Set 6:

(1 comment)

File src/include/option.h:

https://review.coreboot.org/c/coreboot/+/47107/comment/717d2063_00a9c76e PS6, Line 6: #include <types.h>

...

Eh, enum cb_err

Done

-- To view, visit https://review.coreboot.org/c/coreboot/+/47107 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I6bbc51135216f34518cfd05c3dc90fb68404c1cc Gerrit-Change-Number: 47107 Gerrit-PatchSet: 6 Gerrit-Owner: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@mailbox.org Gerrit-Attention: Nico Huber nico.h@gmx.de Gerrit-Attention: Patrick Rudolph siro@das-labor.org Gerrit-Comment-Date: Mon, 19 Apr 2021 17:17:36 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons th3fanbus@gmail.com Gerrit-MessageType: comment

Patrick Rudolph (Code Review)

5:53 p.m.

New subject: Change in coreboot[master]: option.h: Introduce {get,set}_int_option() functions

Attention is currently required from: Nico Huber, Angel Pons. Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47107 )

Change subject: option.h: Introduce {get,set}_int_option() functions ......................................................................

Patch Set 7: Code-Review+2

-- To view, visit https://review.coreboot.org/c/coreboot/+/47107 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I6bbc51135216f34518cfd05c3dc90fb68404c1cc Gerrit-Change-Number: 47107 Gerrit-PatchSet: 7 Gerrit-Owner: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@mailbox.org Gerrit-Attention: Nico Huber nico.h@gmx.de Gerrit-Attention: Angel Pons th3fanbus@gmail.com Gerrit-Comment-Date: Mon, 19 Apr 2021 17:53:09 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Patrick Georgi (Code Review)

21 Apr 21 Apr

8:58 a.m.

New subject: Change in coreboot[master]: option.h: Introduce {get,set}_int_option() functions

Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/47107 )

Change subject: option.h: Introduce {get,set}_int_option() functions ......................................................................

option.h: Introduce {get,set}_int_option() functions

These new functions will be put to use in subsequent commits.

Change-Id: I6bbc51135216f34518cfd05c3dc90fb68404c1cc Signed-off-by: Angel Pons th3fanbus@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/47107 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Patrick Rudolph siro@das-labor.org --- M src/include/option.h 1 file changed, 11 insertions(+), 0 deletions(-)

Approvals: build bot (Jenkins): Verified Patrick Rudolph: Looks good to me, approved

diff --git a/src/include/option.h b/src/include/option.h index 6b4e6a9..26cd0d5 100644 --- a/src/include/option.h +++ b/src/include/option.h @@ -26,4 +26,15 @@ return CB_CMOS_OTABLE_DISABLED; }

+static inline enum cb_err set_int_option(const char *name, int value) +{ + return set_option(name, &value); +} + +static inline int get_int_option(const char *name, const int fallback) +{ + int value = 0; + return get_option(&value, name) == CB_SUCCESS ? value : fallback; +} + #endif /* _OPTION_H_ */

-- To view, visit https://review.coreboot.org/c/coreboot/+/47107 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I6bbc51135216f34518cfd05c3dc90fb68404c1cc Gerrit-Change-Number: 47107 Gerrit-PatchSet: 8 Gerrit-Owner: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@mailbox.org Gerrit-MessageType: merged

1318

days inactive

1488

days old

coreboot-gerrit@coreboot.org

16 comments

5 participants

tags (0)

participants (5)

Angel Pons (Code Review)
build bot (Jenkins) (Code Review)
Nico Huber (Code Review)
Patrick Georgi (Code Review)
Patrick Rudolph (Code Review)