July 2021 - flashrom-gerrit - mail.coreboot.org

Change in flashrom[master]: flashrom.c: Reorder check_block_eraser() to avoid forward decl
by Edward O'Callaghan (Code Review) 16 Jul '21

16 Jul '21

Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/56296 ) Change subject: flashrom.c: Reorder check_block_eraser() to avoid forward decl ...................................................................... flashrom.c: Reorder check_block_eraser() to avoid forward decl Help make groking flashrom.c fractionaly easier. BUG=none BRANCH=none TEST=builds Change-Id: I0906a6e581ce5135b58f6acc6339908dfa770a59 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/56296 Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M flashrom.c 1 file changed, 25 insertions(+), 27 deletions(-) Approvals: build bot (Jenkins): Verified Anastasia Klimchuk: Looks good to me, approved diff --git a/flashrom.c b/flashrom.c index 1f94f33..8d44c3d 100644 --- a/flashrom.c +++ b/flashrom.c @@ -73,8 +73,6 @@ /* Did we change something or was every erase/write skipped (if any)? */ static bool all_skipped = true; -static int check_block_eraser(const struct flashctx *flash, int k, int log); - /* Register a function to be executed on programmer shutdown. * The advantage over atexit() is that you can supply a void pointer which will * be used as parameter to the registered function upon programmer shutdown. @@ -333,6 +331,31 @@ return extract_param(&programmer_param, param_name, ","); } +static int check_block_eraser(const struct flashctx *flash, int k, int log) +{ + struct block_eraser eraser = flash->chip->block_erasers[k]; + + if (!eraser.block_erase && !eraser.eraseblocks[0].count) { + if (log) + msg_cdbg("not defined. "); + return 1; + } + if (!eraser.block_erase && eraser.eraseblocks[0].count) { + if (log) + msg_cdbg("eraseblock layout is known, but matching " + "block erase function is not implemented. "); + return 1; + } + if (eraser.block_erase && !eraser.eraseblocks[0].count) { + if (log) + msg_cdbg("block erase function found, but " + "eraseblock layout is not defined. "); + return 1; + } + // TODO: Once erase functions are annotated with allowed buses, check that as well. + return 0; +} + /* Returns the number of well-defined erasers for a chip. */ static unsigned int count_usable_erasers(const struct flashctx *flash) { @@ -1125,31 +1148,6 @@ return ret; } -static int check_block_eraser(const struct flashctx *flash, int k, int log) -{ - struct block_eraser eraser = flash->chip->block_erasers[k]; - - if (!eraser.block_erase && !eraser.eraseblocks[0].count) { - if (log) - msg_cdbg("not defined. "); - return 1; - } - if (!eraser.block_erase && eraser.eraseblocks[0].count) { - if (log) - msg_cdbg("eraseblock layout is known, but matching " - "block erase function is not implemented. "); - return 1; - } - if (eraser.block_erase && !eraser.eraseblocks[0].count) { - if (log) - msg_cdbg("block erase function found, but " - "eraseblock layout is not defined. "); - return 1; - } - // TODO: Once erase functions are annotated with allowed buses, check that as well. - return 0; -} - /** * @brief Reads the included layout regions into a buffer. * -- To view, visit https://review.coreboot.org/c/flashrom/+/56296 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0906a6e581ce5135b58f6acc6339908dfa770a59 Gerrit-Change-Number: 56296 Gerrit-PatchSet: 3 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

Change in flashrom[master]: flashrom.c: Make extract_param() static local
by Edward O'Callaghan (Code Review) 16 Jul '21

16 Jul '21

Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/56295 ) Change subject: flashrom.c: Make extract_param() static local ...................................................................... flashrom.c: Make extract_param() static local The function is only ever used within flashrom.c. BUG=none BRANCH=none TEST=builds Change-Id: I81f1cdb9df98c151201390edeb69c74defe7881f Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/56295 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Nico Huber <nico.h(a)gmx.de> Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M flash.h M flashrom.c 2 files changed, 1 insertion(+), 2 deletions(-) Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved Anastasia Klimchuk: Looks good to me, approved diff --git a/flash.h b/flash.h index c6a53b1..3baa076 100644 --- a/flash.h +++ b/flash.h @@ -344,7 +344,6 @@ int erase_flash(struct flashctx *flash); int probe_flash(struct registered_master *mst, int startchip, struct flashctx *fill_flash, int force); int read_flash_to_file(struct flashctx *flash, const char *filename); -char *extract_param(const char *const *haystack, const char *needle, const char *delim); int verify_range(struct flashctx *flash, const uint8_t *cmpbuf, unsigned int start, unsigned int len); int need_erase(const uint8_t *have, const uint8_t *want, unsigned int len, enum write_granularity gran, const uint8_t erased_value); void print_version(void); diff --git a/flashrom.c b/flashrom.c index 6cc899f..1f94f33 100644 --- a/flashrom.c +++ b/flashrom.c @@ -272,7 +272,7 @@ * needle and remove everything from the first occurrence of needle to the next * delimiter from haystack. */ -char *extract_param(const char *const *haystack, const char *needle, const char *delim) +static char *extract_param(const char *const *haystack, const char *needle, const char *delim) { char *param_pos, *opt_pos, *rest; char *opt = NULL; -- To view, visit https://review.coreboot.org/c/flashrom/+/56295 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I81f1cdb9df98c151201390edeb69c74defe7881f Gerrit-Change-Number: 56295 Gerrit-PatchSet: 2 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> 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-MessageType: merged

1 0

Change in flashrom[master]: flashrom.c: Reorder check_block_eraser() to avoid forward decl
by Edward O'Callaghan (Code Review) 16 Jul '21

16 Jul '21

Attention is currently required from: Angel Pons. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/56296 ) Change subject: flashrom.c: Reorder check_block_eraser() to avoid forward decl ...................................................................... Patch Set 2: (1 comment) Patchset: PS2: > > curl: (6) Could not resolve host: wrapdb.mesonbuild.com […] I guess the package isn't installed on the buildbot and so meson is sourcing it for the test environment. -- To view, visit https://review.coreboot.org/c/flashrom/+/56296 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0906a6e581ce5135b58f6acc6339908dfa770a59 Gerrit-Change-Number: 56296 Gerrit-PatchSet: 2 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Fri, 16 Jul 2021 02:09:22 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: tests: Wrap strdup to help cmocka recognise memory allocation
by Anastasia Klimchuk (Code Review) 16 Jul '21

16 Jul '21

Attention is currently required from: Nico Huber, Angel Pons. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/56323 ) Change subject: tests: Wrap strdup to help cmocka recognise memory allocation ...................................................................... Patch Set 2: (1 comment) File tests/tests.c: https://review.coreboot.org/c/flashrom/+/56323/comment/861a9493_1007e59a PS1, Line 34: char *__wrap_strdup(const char *s) > Maybe add a comment referencing the cmocka issue? […] Done. Also I decided to remove LOG_ME from __wrap_strdup. As a person who is looking at test logs very often, I realised it just adds noise to the logs. This wrap is special, not like the others. -- To view, visit https://review.coreboot.org/c/flashrom/+/56323 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I56aef6b342752d80995c36ab075b12198fc101d9 Gerrit-Change-Number: 56323 Gerrit-PatchSet: 2 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: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Fri, 16 Jul 2021 00:20:05 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: tests: Wrap strdup to help cmocka recognise memory allocation
by Anastasia Klimchuk (Code Review) 16 Jul '21

16 Jul '21

Attention is currently required from: Nico Huber, Anastasia Klimchuk. Hello build bot (Jenkins), Nico Huber, Edward O'Callaghan, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/56323 to look at the new patch set (#2). Change subject: tests: Wrap strdup to help cmocka recognise memory allocation ...................................................................... tests: Wrap strdup to help cmocka recognise memory allocation This is a known issue in cmocka (see https://github.com/clibs/cmocka/issues/17) where cmocka does not recognise memory allocation happening inside strdup, and then later throws an error when the memory is freed. If the issue is fixed at some point, this workaround can be removed. Given that cmocka already overrides malloc, calloc, realloc, free, adding strdup there seems fine. Existing tests now can (and have to) free the memory they allocated by strdup, and this is in the same patch. BUG=b:193584590 TEST=ninja test Change-Id: I56aef6b342752d80995c36ab075b12198fc101d9 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M tests/init_shutdown.c M tests/meson.build M tests/tests.c 3 files changed, 17 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/23/56323/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/56323 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I56aef6b342752d80995c36ab075b12198fc101d9 Gerrit-Change-Number: 56323 Gerrit-PatchSet: 2 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: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: flashrom.c: Reorder check_block_eraser() to avoid forward decl
by Angel Pons (Code Review) 15 Jul '21

15 Jul '21

Attention is currently required from: Edward O'Callaghan. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/56296 ) Change subject: flashrom.c: Reorder check_block_eraser() to avoid forward decl ...................................................................... Patch Set 2: (1 comment) Patchset: PS2: > curl: (6) Could not resolve host: wrapdb.mesonbuild.com Why is meson trying to access this? -- To view, visit https://review.coreboot.org/c/flashrom/+/56296 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0906a6e581ce5135b58f6acc6339908dfa770a59 Gerrit-Change-Number: 56296 Gerrit-PatchSet: 2 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Thu, 15 Jul 2021 09:40:15 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: tests: Add layout tests for overlapping regions
by Angel Pons (Code Review) 15 Jul '21

15 Jul '21

Attention is currently required from: Nico Huber, Edward O'Callaghan, Anastasia Klimchuk. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/56324 ) Change subject: tests: Add layout tests for overlapping regions ...................................................................... Patch Set 2: Code-Review+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/56324 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7aa8dc0c9bc5a22fe5deea757eea1a151b969cea Gerrit-Change-Number: 56324 Gerrit-PatchSet: 2 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: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-CC: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Thu, 15 Jul 2021 09:38:26 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: tests: Wrap strdup to help cmocka recognise memory allocation
by Angel Pons (Code Review) 15 Jul '21

15 Jul '21

Attention is currently required from: Nico Huber, Anastasia Klimchuk. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/56323 ) Change subject: tests: Wrap strdup to help cmocka recognise memory allocation ...................................................................... Patch Set 1: Code-Review+2 (1 comment) File tests/tests.c: https://review.coreboot.org/c/flashrom/+/56323/comment/ef850e78_055b76df PS1, Line 34: char *__wrap_strdup(const char *s) Maybe add a comment referencing the cmocka issue? /* Workaround for https://github.com/clibs/cmocka/issues/17 */ -- To view, visit https://review.coreboot.org/c/flashrom/+/56323 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I56aef6b342752d80995c36ab075b12198fc101d9 Gerrit-Change-Number: 56323 Gerrit-PatchSet: 1 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: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Thu, 15 Jul 2021 09:37:12 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: tests: Add layout tests for overlapping regions
by Anastasia Klimchuk (Code Review) 15 Jul '21

15 Jul '21

Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/56324 ) Change subject: tests: Add layout tests for overlapping regions ...................................................................... Patch Set 2: (1 comment) File tests/layout.c: https://review.coreboot.org/c/flashrom/+/56324/comment/afd73a0f_bc62d6fb PS1, Line 21: int flashrom_layout_new(struct flashrom_layout **); : int flashrom_layout_add_region(struct flashrom_layout *, size_t start, size_t end, const char *name); : int flashrom_layout_include_region(struct flashrom_layout *, const char *name); : void flashrom_layout_release(struct flashrom_layout *); > what about including `libflashrom. […] It works and looks so much better, thank you! -- To view, visit https://review.coreboot.org/c/flashrom/+/56324 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7aa8dc0c9bc5a22fe5deea757eea1a151b969cea Gerrit-Change-Number: 56324 Gerrit-PatchSet: 2 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: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-CC: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Thu, 15 Jul 2021 02:24:13 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: tests: Add layout tests for overlapping regions
by Anastasia Klimchuk (Code Review) 15 Jul '21

15 Jul '21

Attention is currently required from: Nico Huber, Angel Pons, Anastasia Klimchuk. Hello build bot (Jenkins), Nico Huber, Edward O'Callaghan, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/56324 to look at the new patch set (#2). Change subject: tests: Add layout tests for overlapping regions ...................................................................... tests: Add layout tests for overlapping regions There are no tests for layout, it would be great to add some. Also partially inspired by commit 06a89d713951a2e08ef8fb698a7688357baa83d1 and commit c9039fc27916c03e21ba91365d01e6bc49503053 BUG=b:193584590 TEST=ninja test Change-Id: I7aa8dc0c9bc5a22fe5deea757eea1a151b969cea Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- A tests/layout.c M tests/meson.build M tests/tests.c M tests/tests.h 4 files changed, 116 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/24/56324/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/56324 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7aa8dc0c9bc5a22fe5deea757eea1a151b969cea Gerrit-Change-Number: 56324 Gerrit-PatchSet: 2 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: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-CC: Peter Marheine <pmarheine(a)chromium.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-MessageType: newpatchset

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit July 2021