February 2022 - flashrom-gerrit

Change in flashrom[master]: libflashrom/fmap: Don't use off_t for flash offsets
by Nikolai Artemiev (Code Review) 18 Feb '22

18 Feb '22

Attention is currently required from: Nico Huber, David Hendricks, Julius Werner, Angel Pons, Arthur Heymans. Nikolai Artemiev has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/61943 ) Change subject: libflashrom/fmap: Don't use off_t for flash offsets ...................................................................... Patch Set 2: Code-Review+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/61943 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I68a386973f79ea634f63dfcd7d95a63400e1fdee Gerrit-Change-Number: 61943 Gerrit-PatchSet: 2 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Fri, 18 Feb 2022 04:36:41 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: libflashrom/fmap: Don't use off_t for flash offsets
by Edward O'Callaghan (Code Review) 18 Feb '22

18 Feb '22

Attention is currently required from: Nico Huber, David Hendricks, Julius Werner, Angel Pons, Arthur Heymans. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/61943 ) Change subject: libflashrom/fmap: Don't use off_t for flash offsets ...................................................................... Patch Set 1: Code-Review+2 (1 comment) Patchset: PS1: > ... […] I ran into this problem today from `elogtool list` breaking is a truly bizarre way with "Out of memory". It wasted a fair amount of time until when I saw: ``` flashrom_layout_read_fmap_from_rom: offset=8388608 and len=0. fmap_read_from_rom: offset=0 and len=-178828444. ``` it suddenly clicked that this looked like something familiar ;) This patches resolved the issue. -- To view, visit https://review.coreboot.org/c/flashrom/+/61943 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I68a386973f79ea634f63dfcd7d95a63400e1fdee Gerrit-Change-Number: 61943 Gerrit-PatchSet: 1 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Fri, 18 Feb 2022 04:33:57 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: libflashrom/fmap: Don't use off_t for flash offsets
by Edward O'Callaghan (Code Review) 18 Feb '22

18 Feb '22

Attention is currently required from: Nico Huber, David Hendricks, Julius Werner, Angel Pons, Arthur Heymans. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/61943 ) Change subject: libflashrom/fmap: Don't use off_t for flash offsets ...................................................................... Patch Set 2: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/61943/comment/06af935d_8a829fe6 PS2, Line 19: BUG=b:219811851 TEST=`elogtool list` on cherry. -- To view, visit https://review.coreboot.org/c/flashrom/+/61943 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I68a386973f79ea634f63dfcd7d95a63400e1fdee Gerrit-Change-Number: 61943 Gerrit-PatchSet: 2 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Fri, 18 Feb 2022 04:17:52 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: libflashrom/fmap: Don't use off_t for flash offsets
by Edward O'Callaghan (Code Review) 18 Feb '22

18 Feb '22

Attention is currently required from: Nico Huber, David Hendricks, Julius Werner, Angel Pons, Arthur Heymans. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/61943 ) Change subject: libflashrom/fmap: Don't use off_t for flash offsets ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/61943 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I68a386973f79ea634f63dfcd7d95a63400e1fdee Gerrit-Change-Number: 61943 Gerrit-PatchSet: 2 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Fri, 18 Feb 2022 04:16:19 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: tests: Add tests for verify operation
by Edward O'Callaghan (Code Review) 18 Feb '22

18 Feb '22

Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/59239 ) Change subject: tests: Add tests for verify operation ...................................................................... tests: Add tests for verify operation This patch adds two tests which cover verify operation, and adds io_mock for fread. BUG=b:181803212 TEST=ninja test Change-Id: I1cc6f73f9b1e385eb963adccf20759c13a40ed3b Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/59239 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> Reviewed-by: Felix Singer <felixsinger(a)posteo.net> --- M tests/chip.c M tests/tests.c M tests/tests.h 3 files changed, 100 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Felix Singer: Looks good to me, approved Edward O'Callaghan: Looks good to me, approved diff --git a/tests/chip.c b/tests/chip.c index 798199c..fe12533 100644 --- a/tests/chip.c +++ b/tests/chip.c @@ -31,6 +31,7 @@ #include "chipdrivers.h" #include "flash.h" +#include "io_mock.h" #include "libflashrom.h" #include "programmer.h" @@ -350,3 +351,98 @@ free(param_dup); free(newcontents); } + +size_t verify_chip_fread(void *state, void *buf, size_t size, size_t len, FILE *fp) +{ + /* + * Verify operation compares contents of the file vs contents on the chip. + * To emulate successful verification we emulate file contents to be the + * same as what is on the chip. + */ + memset(buf, MOCK_CHIP_CONTENT, len); + return len; +} + +void verify_chip_test_success(void **state) +{ + (void) state; /* unused */ + + const struct io_mock verify_chip_io = { + .fread = verify_chip_fread, + }; + + io_mock_register(&verify_chip_io); + + struct flashrom_flashctx flashctx = { 0 }; + struct flashrom_layout *layout; + struct flashchip mock_chip = chip_8MiB; + const char *param = ""; /* Default values for all params. */ + + setup_chip(&flashctx, &layout, &mock_chip, param); + + /* See comment in write_chip_test_success */ + const char *const filename = "-"; + unsigned long size = mock_chip.total_size * 1024; + uint8_t *const newcontents = malloc(size); + + printf("Verify chip operation started.\n"); + assert_int_equal(0, read_buf_from_file(newcontents, size, filename)); + assert_int_equal(0, flashrom_image_verify(&flashctx, newcontents, size)); + printf("Verify chip operation done.\n"); + + teardown(&layout); + + free(newcontents); + + io_mock_register(NULL); +} + +void verify_chip_with_dummyflasher_test_success(void **state) +{ + (void) state; /* unused */ + + const struct io_mock verify_chip_io = { + .fread = verify_chip_fread, + }; + + io_mock_register(&verify_chip_io); + + struct flashrom_flashctx flashctx = { 0 }; + struct flashrom_layout *layout; + struct flashchip mock_chip = chip_W25Q128_V; + /* + * Dummyflasher is capable to emulate W25Q128.V, so we ask it to do this. + * Nothing to mock, dummy is taking care of this already. + */ + char *param_dup = strdup("bus=spi,emulate=W25Q128FV"); + + setup_chip(&flashctx, &layout, &mock_chip, param_dup); + + /* See comment in write_chip_test_success */ + const char *const filename = "-"; + unsigned long size = mock_chip.total_size * 1024; + uint8_t *const newcontents = malloc(size); + + /* + * Dummyflasher controls chip state and fully emulates reads and writes, + * so to set up initial chip state we need to write on chip. Write + * operation takes content from file and writes on chip. File content is + * emulated in verify_chip_fread mock. + */ + + printf("Write chip operation started.\n"); + assert_int_equal(0, read_buf_from_file(newcontents, size, filename)); + assert_int_equal(0, flashrom_image_write(&flashctx, newcontents, size, NULL)); + printf("Write chip operation done.\n"); + + printf("Verify chip operation started.\n"); + assert_int_equal(0, flashrom_image_verify(&flashctx, newcontents, size)); + printf("Verify chip operation done.\n"); + + teardown(&layout); + + free(param_dup); + free(newcontents); + + io_mock_register(NULL); +} diff --git a/tests/tests.c b/tests/tests.c index 06dc12f..1dda467 100644 --- a/tests/tests.c +++ b/tests/tests.c @@ -390,6 +390,8 @@ cmocka_unit_test(read_chip_with_dummyflasher_test_success), cmocka_unit_test(write_chip_test_success), cmocka_unit_test(write_chip_with_dummyflasher_test_success), + cmocka_unit_test(verify_chip_test_success), + cmocka_unit_test(verify_chip_with_dummyflasher_test_success), }; ret |= cmocka_run_group_tests_name("chip.c tests", chip_tests, NULL, NULL); diff --git a/tests/tests.h b/tests/tests.h index 16974af..f1b4e9b 100644 --- a/tests/tests.h +++ b/tests/tests.h @@ -64,5 +64,7 @@ void read_chip_with_dummyflasher_test_success(void **state); void write_chip_test_success(void **state); void write_chip_with_dummyflasher_test_success(void **state); +void verify_chip_test_success(void **state); +void verify_chip_with_dummyflasher_test_success(void **state); #endif /* TESTS_H */ -- To view, visit https://review.coreboot.org/c/flashrom/+/59239 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I1cc6f73f9b1e385eb963adccf20759c13a40ed3b Gerrit-Change-Number: 59239 Gerrit-PatchSet: 9 Gerrit-Owner: 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: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged

1 0

Change in flashrom[master]: tests: Add tests for verify operation
by Edward O'Callaghan (Code Review) 18 Feb '22

18 Feb '22

Attention is currently required from: Nico Huber, Angel Pons, Anastasia Klimchuk. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/59239 ) Change subject: tests: Add tests for verify operation ...................................................................... Patch Set 8: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/59239 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I1cc6f73f9b1e385eb963adccf20759c13a40ed3b Gerrit-Change-Number: 59239 Gerrit-PatchSet: 8 Gerrit-Owner: 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: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Fri, 18 Feb 2022 03:39:36 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: libflashrom/fmap: Don't use off_t for flash offsets
by Julius Werner (Code Review) 18 Feb '22

18 Feb '22

Attention is currently required from: Nico Huber, David Hendricks, Angel Pons, Arthur Heymans. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/61943 ) Change subject: libflashrom/fmap: Don't use off_t for flash offsets ...................................................................... Patch Set 2: (2 comments) File fmap.c: https://review.coreboot.org/c/flashrom/+/61943/comment/188c5e0b_f73f1933 PS1, Line 94: int32_t > Thoughts on using `ssize_t` instead? Done File libflashrom.h: https://review.coreboot.org/c/flashrom/+/61943/comment/8615cd94_4e4dc337 PS1, Line 112: uint32_t > +1 Done -- To view, visit https://review.coreboot.org/c/flashrom/+/61943 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I68a386973f79ea634f63dfcd7d95a63400e1fdee Gerrit-Change-Number: 61943 Gerrit-PatchSet: 2 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Fri, 18 Feb 2022 01:39:59 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: libflashrom/fmap: Don't use off_t for flash offsets
by Julius Werner (Code Review) 18 Feb '22

18 Feb '22

Attention is currently required from: Nico Huber, David Hendricks, Julius Werner, Arthur Heymans. Hello build bot (Jenkins), Nico Huber, David Hendricks, Edward O'Callaghan, Angel Pons, Arthur Heymans, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/61943 to look at the new patch set (#2). Change subject: libflashrom/fmap: Don't use off_t for flash offsets ...................................................................... libflashrom/fmap: Don't use off_t for flash offsets off_t is a special POSIX type that is used to represent file offsets in certain APIs (e.g. lseek(), mmap()), and should not be reused to represent anything else (such as flash offsets). In particular, the width of the type may change based on the definition of the _FILE_OFFSET_BITS macro. Using such a type at the libflashrom interface is particularly dangerous, because if a program is built with a different _FILE_OFFSET_BITS value than libflashrom, the resulting ABI corruption will cause very very nasty and confusing bugs. This patch replaces all instances of off_t that are not related to file offsets with (s)size_t. Signed-off-by: Julius Werner <jwerner(a)chromium.org> Change-Id: I68a386973f79ea634f63dfcd7d95a63400e1fdee --- M fmap.c M libflashrom.c M libflashrom.h 3 files changed, 6 insertions(+), 6 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/43/61943/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/61943 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I68a386973f79ea634f63dfcd7d95a63400e1fdee Gerrit-Change-Number: 61943 Gerrit-PatchSet: 2 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: flashchips,writeprotect_ranges: add range decoding function
by Nico Huber (Code Review) 17 Feb '22

17 Feb '22

Attention is currently required from: Angel Pons, Nikolai Artemiev, Sergii Dmytruk. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58480 ) Change subject: flashchips,writeprotect_ranges: add range decoding function ...................................................................... Patch Set 35: Code-Review+1 (3 comments) File flash.h: https://review.coreboot.org/c/flashrom/+/58480/comment/64ae2632_f4e8bb88 PS35, Line 311: size_t *start, size_t *len Should we put output parameters first? File writeprotect_ranges.c: https://review.coreboot.org/c/flashrom/+/58480/comment/85f64148_89ad2fa1 PS35, Line 8: * the Free Software Foundation; version 2 of the License. Can we have v2+? https://review.coreboot.org/c/flashrom/+/58480/comment/4451d3b8_61283d57 PS35, Line 23: * various chips is more fully known. I guess this would be something that's never 100% finished. Maybe not worth a TODO. -- To view, visit https://review.coreboot.org/c/flashrom/+/58480 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id163ed80938a946a502ed116e48e8236e36eb203 Gerrit-Change-Number: 58480 Gerrit-PatchSet: 35 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> 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: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Comment-Date: Thu, 17 Feb 2022 18:12:56 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: libflashrom,writeprotect: add functions for reading/writing WP configs
by Nico Huber (Code Review) 17 Feb '22

17 Feb '22

Attention is currently required from: Edward O'Callaghan, Nikolai Artemiev, Sergii Dmytruk. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58479 ) Change subject: libflashrom,writeprotect: add functions for reading/writing WP configs ...................................................................... Patch Set 34: (7 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/58479/comment/3599d703_9f7602c9 PS34, Line 11: a spurious `a` File libflashrom.h: https://review.coreboot.org/c/flashrom/+/58479/comment/0a94607f_f13bb936 PS34, Line 125: FLASHROM_WP_OK = 0, NB. we should unify this across the whole API sooner or later. https://review.coreboot.org/c/flashrom/+/58479/comment/24decda6_5743a364 PS34, Line 151: excessive empty line File libflashrom.c: https://review.coreboot.org/c/flashrom/+/58479/comment/5674ad61_45e26c0b PS34, Line 642: struct flashrom_wp_cfg Or `sizeof(**cfg)` https://review.coreboot.org/c/flashrom/+/58479/comment/8d3d4189_1ba933ec PS34, Line 643: (*cfg) Nit, no parentheses needed. https://review.coreboot.org/c/flashrom/+/58479/comment/9444f0b3_905b5e56 PS34, Line 717: * has its own WP operations we should use instead. Shouldn't we check the type right away and return an error in case? Or check inside wp_write_cfg()? Currently it relies on `flash->chip-> bus_type == BUS_SPI` and matching contents of `flash->chip->reg_bits`. File writeprotect.c: https://review.coreboot.org/c/flashrom/+/58479/comment/9b42faee_09a3fa74 PS34, Line 51: = FLASHROM_WP_OK This is overwritten on the very next line. -- To view, visit https://review.coreboot.org/c/flashrom/+/58479 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3ad25708c3321b8fb0216c3eaf6ffc07616537ad Gerrit-Change-Number: 58479 Gerrit-PatchSet: 34 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> 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: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Comment-Date: Thu, 17 Feb 2022 18:12:20 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit February 2022