September 2022 - flashrom-gerrit

[S] Change in flashrom[master]: programmer_init: Work on a mutable copy of programmer params
by Edward O'Callaghan (Code Review) 14 Sep '22

14 Sep '22

Attention is currently required from: Felix Singer, Nico Huber, Thomas Heijligen, Angel Pons, Anastasia Klimchuk. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67094 ) Change subject: programmer_init: Work on a mutable copy of programmer params ...................................................................... Patch Set 3: (1 comment) File flashrom.c: https://review.coreboot.org/c/flashrom/+/67094/comment/4e1d8c86_e2eebb09 PS3, Line 157: -1 probably `ERROR_FATAL` to be consistent with return codes with the rest of the function. -- To view, visit https://review.coreboot.org/c/flashrom/+/67094 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: If6bb2e5e4312b07f756615984bd3757e92b86b0a Gerrit-Change-Number: 67094 Gerrit-PatchSet: 3 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> 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: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Wed, 14 Sep 2022 00:33:50 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: programmer_init: Work on a mutable copy of programmer params
by Edward O'Callaghan (Code Review) 14 Sep '22

14 Sep '22

Attention is currently required from: Felix Singer, Nico Huber, Thomas Heijligen, Angel Pons, Anastasia Klimchuk. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67094 ) Change subject: programmer_init: Work on a mutable copy of programmer params ...................................................................... Patch Set 3: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/67094 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: If6bb2e5e4312b07f756615984bd3757e92b86b0a Gerrit-Change-Number: 67094 Gerrit-PatchSet: 3 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> 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: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Wed, 14 Sep 2022 00:12:25 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: Revert "flashrom.c: Remove programmer_param global state"
by Edward O'Callaghan (Code Review) 14 Sep '22

14 Sep '22

Attention is currently required from: Thomas Heijligen. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67621 ) Change subject: Revert "flashrom.c: Remove programmer_param global state" ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: > Oh I didn't know it's a valid use case to invoke initialisation with NULL prog params :\ But now I k […] Thanks for spotting this corner case and the revert Thomas! Yes great idea to get this covered by the test too! Feel free to merge right away! -- To view, visit https://review.coreboot.org/c/flashrom/+/67621 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3b689ad4bdd0c9c3b11f30becafc878c78630f0b Gerrit-Change-Number: 67621 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Jonathon Hall <jonathon.hall(a)puri.sm> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Comment-Date: Wed, 14 Sep 2022 00:11:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: Revert "flashrom.c: Remove programmer_param global state"
by Edward O'Callaghan (Code Review) 14 Sep '22

14 Sep '22

Attention is currently required from: Thomas Heijligen. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67621 ) Change subject: Revert "flashrom.c: Remove programmer_param global state" ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/67621 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3b689ad4bdd0c9c3b11f30becafc878c78630f0b Gerrit-Change-Number: 67621 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Jonathon Hall <jonathon.hall(a)puri.sm> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Comment-Date: Wed, 14 Sep 2022 00:10:03 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: Revert "flashrom.c: Remove programmer_param global state"
by Anastasia Klimchuk (Code Review) 13 Sep '22

13 Sep '22

Attention is currently required from: Thomas Heijligen, Edward O'Callaghan. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67621 ) Change subject: Revert "flashrom.c: Remove programmer_param global state" ...................................................................... Patch Set 1: Code-Review+2 (1 comment) Patchset: PS1: Oh I didn't know it's a valid use case to invoke initialisation with NULL prog params :\ But now I know, and this should be covered by unit test. I created one in CB:67642, currently fails at head and passes on the top of this revert. Test can be merged after the revert, and then whatever is another version of removing global programmer_param, test will run over it. -- To view, visit https://review.coreboot.org/c/flashrom/+/67621 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3b689ad4bdd0c9c3b11f30becafc878c78630f0b Gerrit-Change-Number: 67621 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Jonathon Hall <jonathon.hall(a)puri.sm> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.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: Tue, 13 Sep 2022 23:14:42 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: tests: Add unit test for initialisation with NULL programmer param
by Felix Singer (Code Review) 13 Sep '22

13 Sep '22

Attention is currently required from: Thomas Heijligen, Angel Pons, Anastasia Klimchuk. Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67642 ) Change subject: tests: Add unit test for initialisation with NULL programmer param ...................................................................... Patch Set 1: Code-Review+1 (1 comment) Patchset: PS1: > This new test fails at head at the moment of writing (as expected), however I applied revert CB:6762 […] How about rebasing on CB:67621? -- To view, visit https://review.coreboot.org/c/flashrom/+/67642 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I409f1c9ac832943e54107f7cf8652d1f46ac67df Gerrit-Change-Number: 67642 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Tue, 13 Sep 2022 23:14:00 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: tests: Add unit test for initialisation with NULL programmer param
by Anastasia Klimchuk (Code Review) 13 Sep '22

13 Sep '22

Attention is currently required from: Felix Singer, Thomas Heijligen, Angel Pons. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67642 ) Change subject: tests: Add unit test for initialisation with NULL programmer param ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: This new test fails at head at the moment of writing (as expected), however I applied revert CB:67621 locally and test passes on the top of it. Test can be merged after revert is merged. -- To view, visit https://review.coreboot.org/c/flashrom/+/67642 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I409f1c9ac832943e54107f7cf8652d1f46ac67df Gerrit-Change-Number: 67642 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Tue, 13 Sep 2022 23:10:53 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: tests: Add unit test for initialisation with NULL programmer param
by Anastasia Klimchuk (Code Review) 13 Sep '22

13 Sep '22

Anastasia Klimchuk has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/67642 ) Change subject: tests: Add unit test for initialisation with NULL programmer param ...................................................................... tests: Add unit test for initialisation with NULL programmer param Programmer param can be NULL and this is a valid case which can be covered by unit test. `run_lifecycle` needs an adjustment to handle NULL as programmer param, which is also included in the patch. Change-Id: I409f1c9ac832943e54107f7cf8652d1f46ac67df Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M tests/dummyflasher.c M tests/lifecycle.c M tests/tests.c M tests/tests.h 4 files changed, 38 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/42/67642/1 diff --git a/tests/dummyflasher.c b/tests/dummyflasher.c index b52a542..1148217 100644 --- a/tests/dummyflasher.c +++ b/tests/dummyflasher.c @@ -110,6 +110,19 @@ "bus=spi,emulate=W25Q128FV,voltage=3.5V", ERROR_FATAL); } +void dummy_null_prog_param_test_success(void **state) +{ + struct io_mock_fallback_open_state dummy_fallback_open_state = { + .noc = 0, + .paths = { NULL }, + }; + const struct io_mock dummy_io = { + .fallback_open_state = &dummy_fallback_open_state, + }; + + run_basic_lifecycle(state, &dummy_io, &programmer_dummy, NULL); +} + #else SKIP_TEST(dummy_basic_lifecycle_test_success) SKIP_TEST(dummy_probe_lifecycle_test_success) @@ -117,4 +130,5 @@ SKIP_TEST(dummy_init_fails_unhandled_param_test_success) SKIP_TEST(dummy_init_success_invalid_param_test_success) SKIP_TEST(dummy_init_success_unhandled_param_test_success) + SKIP_TEST(dummy_null_prog_param_test_success) #endif /* CONFIG_DUMMY */ diff --git a/tests/lifecycle.c b/tests/lifecycle.c index 568b179..0415757 100644 --- a/tests/lifecycle.c +++ b/tests/lifecycle.c @@ -39,7 +39,12 @@ io_mock_register(io); struct flashrom_programmer *flashprog; - char *param_dup = strdup(param); + char *param_dup; + + if (param != NULL) + param_dup = strdup(param); + else + param_dup = NULL; printf("Testing flashrom_programmer_init for programmer=%s ...\n", prog->name); assert_int_equal(0, flashrom_programmer_init(&flashprog, prog->name, param_dup)); diff --git a/tests/tests.c b/tests/tests.c index 8e3fdde..88c8414 100644 --- a/tests/tests.c +++ b/tests/tests.c @@ -413,6 +413,7 @@ cmocka_unit_test(dummy_init_fails_unhandled_param_test_success), cmocka_unit_test(dummy_init_success_invalid_param_test_success), cmocka_unit_test(dummy_init_success_unhandled_param_test_success), + cmocka_unit_test(dummy_null_prog_param_test_success), cmocka_unit_test(nicrealtek_basic_lifecycle_test_success), cmocka_unit_test(raiden_debug_basic_lifecycle_test_success), cmocka_unit_test(dediprog_basic_lifecycle_test_success), diff --git a/tests/tests.h b/tests/tests.h index cdbf01c..7d84101 100644 --- a/tests/tests.h +++ b/tests/tests.h @@ -48,6 +48,7 @@ void dummy_init_fails_unhandled_param_test_success(void **state); void dummy_init_success_invalid_param_test_success(void **state); void dummy_init_success_unhandled_param_test_success(void **state); +void dummy_null_prog_param_test_success(void **state); void nicrealtek_basic_lifecycle_test_success(void **state); void raiden_debug_basic_lifecycle_test_success(void **state); void dediprog_basic_lifecycle_test_success(void **state); -- To view, visit https://review.coreboot.org/c/flashrom/+/67642 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I409f1c9ac832943e54107f7cf8652d1f46ac67df Gerrit-Change-Number: 67642 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newchange

1 0

[S] Change in flashrom[master]: test_build.sh: Delete old build dir when Meson is used
by Thomas Heijligen (Code Review) 13 Sep '22

13 Sep '22

Attention is currently required from: Felix Singer. Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67541 ) Change subject: test_build.sh: Delete old build dir when Meson is used ...................................................................... Patch Set 3: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/67541 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I050ed916685728cbed9c10601a6bcad6d59a4eda Gerrit-Change-Number: 67541 Gerrit-PatchSet: 3 Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Comment-Date: Tue, 13 Sep 2022 19:13:24 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: print.c: Fix format string for Windows
by Thomas Heijligen (Code Review) 13 Sep '22

13 Sep '22

Thomas Heijligen has submitted this change. ( https://review.coreboot.org/c/flashrom/+/66547 ) Change subject: print.c: Fix format string for Windows ...................................................................... print.c: Fix format string for Windows dwMajorVersion and dwMinorVersion are DWORDS. Cast them to `unsigned long` to avoid a format error. TEST=Builds on Cygwin (Windows 10, amd64, gcc 11.3.0) Change-Id: I049220089f42e488fcc88e6738522f64171720eb Signed-off-by: Thomas Heijligen <thomas.heijligen(a)secunet.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/66547 Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Felix Singer <felixsinger(a)posteo.net> --- M print.c 1 file changed, 21 insertions(+), 1 deletion(-) Approvals: build bot (Jenkins): Verified Felix Singer: Looks good to me, approved Anastasia Klimchuk: Looks good to me, approved diff --git a/print.c b/print.c index 853ef09..98d3109 100644 --- a/print.c +++ b/print.c @@ -524,7 +524,8 @@ /* Tell Windows which version of the structure we want. */ osvi.dwOSVersionInfoSize = sizeof(OSVERSIONINFOEX); if (GetVersionEx((OSVERSIONINFO*) &osvi)) - msg_ginfo(" %lu.%lu", osvi.dwMajorVersion, osvi.dwMinorVersion); + msg_ginfo(" %lu.%lu", (unsigned long)osvi.dwMajorVersion, + (unsigned long)osvi.dwMinorVersion); else msg_ginfo(" unknown version"); GetSystemInfo(&si); -- To view, visit https://review.coreboot.org/c/flashrom/+/66547 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I049220089f42e488fcc88e6738522f64171720eb Gerrit-Change-Number: 66547 Gerrit-PatchSet: 7 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit September 2022