April 2023 - flashrom-gerrit - mail.coreboot.org

[M] Change in flashrom[master]: tests: Emulate multithreading environment for unit tests
by Anastasia Klimchuk (Code Review) 04 Apr '23

04 Apr '23

Attention is currently required from: Thomas Heijligen, Peter Marheine. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/74157 ) Change subject: tests: Emulate multithreading environment for unit tests ...................................................................... Patch Set 5: (1 comment) Patchset: PS2: > Yes, I'd prefer to always create a thread and get rid of the `__isthreaded` poke for other BSDs. […] Finally, it seems like we got to the other side of rabbit hole! :) I tested the version without pre-processor conditionals, it works! One more patch is needed though, I pushed it as a previous one for the chain CB:74202 but otherwise looks like that's it? Peter thank you so much, your help is invaluable! -- To view, visit https://review.coreboot.org/c/flashrom/+/74157 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3d65c125183e60037ad07b9d54b8fffdece5a4e8 Gerrit-Change-Number: 74157 Gerrit-PatchSet: 5 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Idwer Vollering <vidwer(a)gmail.com> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Tue, 04 Apr 2023 13:17:54 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Comment-In-Reply-To: Peter Marheine <pmarheine(a)chromium.org> Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: tests: Emulate multithreading environment for unit tests
by Anastasia Klimchuk (Code Review) 04 Apr '23

04 Apr '23

Attention is currently required from: Thomas Heijligen, Anastasia Klimchuk. Hello build bot (Jenkins), Thomas Heijligen, Peter Marheine, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/74157 to look at the new patch set (#5). Change subject: tests: Emulate multithreading environment for unit tests ...................................................................... tests: Emulate multithreading environment for unit tests The main purpose of this patch is to run unit tests on BSD family of OSes. The root cause is `fileno` syscall which is a macro that can be expanded to either a function call (for multi-threaded environment) or to inline code (for single-threaded environment). Said inline code accesses private field of file descriptor, and this construction is impossible to mock in unit tests. Multi- threaded environment has `fileno` as a function, which can be mocked in unit tests. On other OSes the patch just creates a thread which is doing nothing. We avoid adding pre-processor conditionals since the cost is small. Tested on FreeBSD 13.1-RELEASE-p6 GENERIC amd64 NetBSD 9.2 (GENERIC) amd64 OpenBSD 7.2 GENERIC#7 amd64 DragonFly v6.4.0-RELEASE x86_64 Ubuntu 22.04.1 x86_64 Change-Id: I3d65c125183e60037ad07b9d54b8fffdece5a4e8 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M tests/meson.build M tests/tests.c 2 files changed, 51 insertions(+), 12 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/57/74157/5 -- To view, visit https://review.coreboot.org/c/flashrom/+/74157 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3d65c125183e60037ad07b9d54b8fffdece5a4e8 Gerrit-Change-Number: 74157 Gerrit-PatchSet: 5 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

[M] Change in flashrom[master]: tests: Emulate multithreading environment for unit tests
by Anastasia Klimchuk (Code Review) 04 Apr '23

04 Apr '23

Attention is currently required from: Thomas Heijligen, Anastasia Klimchuk. Hello build bot (Jenkins), Thomas Heijligen, Peter Marheine, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/74157 to look at the new patch set (#4). Change subject: tests: Emulate multithreading environment for unit tests ...................................................................... tests: Emulate multithreading environment for unit tests The main purpose of this patch is to run unit tests on BSD family of OSes. The root cause is `fileno` syscall which is a macro that can be expanded to either a function call (for multi-threaded environment) or to inline code (for single-threaded environment). Said inline code accesses private field of file descriptor, and this construction is impossible to mock in unit tests. Multi- threaded environment has `fileno` as a function, which can be mocked in unit tests. On other OSes the patch just creates a thread which is doing nothing. We avoid adding pre-processor conditionals since the cost is small. Tested on FreeBSD 13.1-RELEASE-p6 GENERIC amd64 NetBSD 9.2 (GENERIC) amd64 OpenBSD 7.2 GENERIC#7 amd64 [WIP] DragonFly v6.4.0-RELEASE x86_64 Ubuntu 22.04.1 x86_64 Change-Id: I3d65c125183e60037ad07b9d54b8fffdece5a4e8 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M tests/meson.build M tests/tests.c 2 files changed, 51 insertions(+), 12 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/57/74157/4 -- To view, visit https://review.coreboot.org/c/flashrom/+/74157 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3d65c125183e60037ad07b9d54b8fffdece5a4e8 Gerrit-Change-Number: 74157 Gerrit-PatchSet: 4 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

[S] Change in flashrom[master]: tests: Fix mode_t argument conversion for va_arg
by Anastasia Klimchuk (Code Review) 04 Apr '23

04 Apr '23

Anastasia Klimchuk has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/74202 ) Change subject: tests: Fix mode_t argument conversion for va_arg ...................................................................... tests: Fix mode_t argument conversion for va_arg Patch fixes the error: error: second argument to 'va_arg' is of promotable type 'mode_t' (aka 'unsigned short'); this va_arg has undefined behavior because arguments will be promoted to 'int' [-Werror,-Wvarargs] Discovered and tested on: FreeBSD clang version 13.0.0 gcc 8.3.0 "cc 8.3 [DragonFly] Release/2019-02-22" Also tested on: gcc 11.3.0 "cc (Ubuntu 11.3.0-1ubuntu1~22.04) 11.3.0" Change-Id: I95b7c8dafdf4e7664c48a952acd7f8eaedb59ba7 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M tests/tests.c 1 file changed, 30 insertions(+), 9 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/02/74202/1 diff --git a/tests/tests.c b/tests/tests.c index d296a98..159b79f 100644 --- a/tests/tests.c +++ b/tests/tests.c @@ -101,40 +101,40 @@ int __wrap_open(const char *pathname, int flags, ...) { LOG_ME; - mode_t mode = 0; + int mode = 0; if (flags & O_CREAT) { va_list ap; va_start(ap, flags); - mode = va_arg(ap, mode_t); + mode = va_arg(ap, int); va_end(ap); } - return mock_open(pathname, flags, mode); + return mock_open(pathname, flags, (mode_t) mode); } int __wrap_open64(const char *pathname, int flags, ...) { LOG_ME; - mode_t mode = 0; + int mode = 0; if (flags & O_CREAT) { va_list ap; va_start(ap, flags); - mode = va_arg(ap, mode_t); + mode = va_arg(ap, int); va_end(ap); } - return mock_open(pathname, flags, mode); + return mock_open(pathname, flags, (mode_t) mode); } int __wrap___open64_2(const char *pathname, int flags, ...) { LOG_ME; - mode_t mode = 0; + int mode = 0; if (flags & O_CREAT) { va_list ap; va_start(ap, flags); - mode = va_arg(ap, mode_t); + mode = va_arg(ap, int); va_end(ap); } - return mock_open(pathname, flags, mode); + return mock_open(pathname, flags, (mode_t) mode); } int __wrap_ioctl(int fd, unsigned long int request, ...) -- To view, visit https://review.coreboot.org/c/flashrom/+/74202 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I95b7c8dafdf4e7664c48a952acd7f8eaedb59ba7 Gerrit-Change-Number: 74202 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newchange

1 0

[M] Change in flashrom[master]: tests: Emulate multithreading environment for unit tests
by Anastasia Klimchuk (Code Review) 04 Apr '23

04 Apr '23

Attention is currently required from: Thomas Heijligen, Anastasia Klimchuk. Hello build bot (Jenkins), Thomas Heijligen, Peter Marheine, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/74157 to look at the new patch set (#3). Change subject: tests: Emulate multithreading environment for unit tests ...................................................................... tests: Emulate multithreading environment for unit tests The main purpose of this patch is to run unit tests on BSD family of OSes. The root cause is `fileno` syscall which is a macro that can be expanded to either a function call (for multi-threaded environment) or to inline code (for single-threaded environment). Said inline code accesses private field of file descriptor, and this construction is impossible to mock in unit tests. Multi- threaded environment has `fileno` as a function, which can be mocked in unit tests. On other OSes the patch just creates a thread which is doing nothing. We avoid adding pre-processor conditionals since the cost is small. Tested on FreeBSD 13.1-RELEASE-p6 GENERIC amd64 NetBSD 9.2 (GENERIC) amd64 OpenBSD 7.2 GENERIC#7 amd64 [WIP] DragonFly v6.4.0-RELEASE x86_64 Ubuntu 22.04.1 x86_64 Change-Id: I3d65c125183e60037ad07b9d54b8fffdece5a4e8 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M tests/meson.build M tests/tests.c 2 files changed, 49 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/57/74157/3 -- To view, visit https://review.coreboot.org/c/flashrom/+/74157 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3d65c125183e60037ad07b9d54b8fffdece5a4e8 Gerrit-Change-Number: 74157 Gerrit-PatchSet: 3 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

[L] Change in flashrom[master]: util/flashchips_db_parser: Add initial rust flashchip json parser
by Edward O'Callaghan (Code Review) 04 Apr '23

04 Apr '23

Attention is currently required from: Nikolai Artemiev. Hello build bot (Jenkins), Nikolai Artemiev, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/74166 to look at the new patch set (#2). Change subject: util/flashchips_db_parser: Add initial rust flashchip json parser ...................................................................... util/flashchips_db_parser: Add initial rust flashchip json parser Change-Id: I48346a3da5923e614728a1cc0567ed2bb1ba2e2f Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- A util/flashchips_db_parser/Cargo.lock A util/flashchips_db_parser/Cargo.toml A util/flashchips_db_parser/src/main.rs A util/flashchips_db_parser/src/parser.rs 4 files changed, 438 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/66/74166/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/74166 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I48346a3da5923e614728a1cc0567ed2bb1ba2e2f Gerrit-Change-Number: 74166 Gerrit-PatchSet: 2 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-MessageType: newpatchset

1 0

[S] Change in flashrom[master]: [WIP] Emulate multithreading env for unit tests on OpenBSD
by Peter Marheine (Code Review) 04 Apr '23

04 Apr '23

Attention is currently required from: Thomas Heijligen, Anastasia Klimchuk. Peter Marheine has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/74157 ) Change subject: [WIP] Emulate multithreading env for unit tests on OpenBSD ...................................................................... Patch Set 2: (2 comments) Patchset: PS2: > Good news: tests pass on OpenBSD with the patchset2! […] Yes, I'd prefer to always create a thread and get rid of the `__isthreaded` poke for other BSDs. It seems fine to also spawn a do-nothing thread on platforms where we don't need to, since the cost is small. File tests/meson.build: https://review.coreboot.org/c/flashrom/+/74157/comment/f9accb29_aef3d6bb PS1, Line 124: '-Dpthread', > Thank you! […] Might be that it ends up included through a pthread header; I fixed an error relating to that by adding the include before I fixed the pthread errors. If it works without changing the includes I don't think it needs any changes. -- To view, visit https://review.coreboot.org/c/flashrom/+/74157 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3d65c125183e60037ad07b9d54b8fffdece5a4e8 Gerrit-Change-Number: 74157 Gerrit-PatchSet: 2 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Mon, 03 Apr 2023 23:32:34 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Comment-In-Reply-To: Peter Marheine <pmarheine(a)chromium.org> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: [WIP] Emulate multithreading env for unit tests on OpenBSD
by Anastasia Klimchuk (Code Review) 03 Apr '23

03 Apr '23

Attention is currently required from: Thomas Heijligen, Peter Marheine. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/74157 ) Change subject: [WIP] Emulate multithreading env for unit tests on OpenBSD ...................................................................... Patch Set 2: (3 comments) This change is ready for review. Patchset: PS2: Good news: tests pass on OpenBSD with the patchset2! I want however to try and reduce the number of `#` here, maybe merge all BSDs hacks into one, maybe hiding them in separate c file. A wilder idea: maybe we can just always unconditionally create "doing nothing" thread (for all OSes), how bad that would be? Which would be the same code but without `#` File tests/meson.build: https://review.coreboot.org/c/flashrom/+/74157/comment/4d5c538b_9d86f85d PS1, Line 114: '-lpthread', > The meson documentation says you should use `dependency('threads')` for this: https://mesonbuild. […] Peter, thank you so much, this really helped! this is what was missing! https://review.coreboot.org/c/flashrom/+/74157/comment/1ae61daa_256e234e PS1, Line 124: '-Dpthread', > This is causing your problem; it makes the preprocessor expand `struct pthread;` to `struct ;`. […] Thank you! For some reason, tests pass even without including `sys/time.h`. Could that be because it is already included via some other include? -- To view, visit https://review.coreboot.org/c/flashrom/+/74157 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3d65c125183e60037ad07b9d54b8fffdece5a4e8 Gerrit-Change-Number: 74157 Gerrit-PatchSet: 2 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Mon, 03 Apr 2023 11:44:49 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Peter Marheine <pmarheine(a)chromium.org> Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: internal: Move laptop_ok into board_cfg
by Sam McNally (Code Review) 03 Apr '23

03 Apr '23

Attention is currently required from: Thomas Heijligen, Edward O'Callaghan, Angel Pons. Sam McNally has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/73456 ) Change subject: internal: Move laptop_ok into board_cfg ...................................................................... Patch Set 5: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/73456 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I459215253845c2af73262943ce91a36464e9eb06 Gerrit-Change-Number: 73456 Gerrit-PatchSet: 5 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Sam McNally <sammc(a)google.com> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Mon, 03 Apr 2023 00:58:27 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: tree/: Case write_granularity enum values
by Angel Pons (Code Review) 02 Apr '23

02 Apr '23

Attention is currently required from: Thomas Heijligen, Edward O'Callaghan. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/74165 ) Change subject: tree/: Case write_granularity enum values ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/74165 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ic8c655225abe477c1b618dc685b743e691c16ebd Gerrit-Change-Number: 74165 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Sun, 02 Apr 2023 16:17:25 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit April 2023