October 2022 - flashrom-gerrit

[S] Change in flashrom[master]: layout.c: Use calloc() to ensure a zeroed layout
by Edward O'Callaghan (Code Review) 11 Oct '22

11 Oct '22

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/68279 ) Change subject: layout.c: Use calloc() to ensure a zeroed layout ...................................................................... layout.c: Use calloc() to ensure a zeroed layout No need to malloc() and then do a DIY memset to zero of the heap. Just use calloc(1, ..) to get a zeroed heap. Change-Id: Id6cf2c4591aec0620f15d8a39495d2bff6597f96 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M layout.c 1 file changed, 14 insertions(+), 3 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/79/68279/1 diff --git a/layout.c b/layout.c index 0212699..be88428 100644 --- a/layout.c +++ b/layout.c @@ -355,14 +355,12 @@ int flashrom_layout_new(struct flashrom_layout **const layout) { - *layout = malloc(sizeof(**layout)); + *layout = calloc(1, sizeof(**layout)); if (!*layout) { msg_gerr("Error creating layout: %s\n", strerror(errno)); return 1; } - const struct flashrom_layout tmp = { 0 }; - **layout = tmp; return 0; } -- To view, visit https://review.coreboot.org/c/flashrom/+/68279 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id6cf2c4591aec0620f15d8a39495d2bff6597f96 Gerrit-Change-Number: 68279 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

1 0

[S] Change in flashrom[master]: flashrom.c: Confine is_internal checks under a symbol
by Edward O'Callaghan (Code Review) 11 Oct '22

11 Oct '22

Attention is currently required from: Felix Singer, Thomas Heijligen, Edward O'Callaghan, Anastasia Klimchuk. Hello Felix Singer, build bot (Jenkins), Thomas Heijligen, Angel Pons, Anastasia Klimchuk, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/67475 to look at the new patch set (#4). Change subject: flashrom.c: Confine is_internal checks under a symbol ...................................................................... flashrom.c: Confine is_internal checks under a symbol We wish for printers to be parametric on the flag state of if we are internal or not and not embed this side-effect. Change-Id: I569811798bfe310c59b8b61afba359bef68969fb Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M flashrom.c 1 file changed, 21 insertions(+), 8 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/75/67475/4 -- To view, visit https://review.coreboot.org/c/flashrom/+/67475 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I569811798bfe310c59b8b61afba359bef68969fb Gerrit-Change-Number: 67475 Gerrit-PatchSet: 4 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: 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-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

[S] Change in flashrom[master]: flashrom.c: Drop emergency_help_message() after erasure
by Edward O'Callaghan (Code Review) 11 Oct '22

11 Oct '22

Attention is currently required from: Felix Singer, Nico Huber, Thomas Heijligen, Angel Pons. Hello Felix Singer, build bot (Jenkins), Thomas Heijligen, Angel Pons, Anastasia Klimchuk, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/67474 to look at the new patch set (#4). Change subject: flashrom.c: Drop emergency_help_message() after erasure ...................................................................... flashrom.c: Drop emergency_help_message() after erasure The emergency help message is only relevent under one specific sitution: i ) The internal programmer was used, && ii) The user was intending not to do a destructive operation. Since erasing the chip is inherantly a destructive operation that was requested warning the user they have a unknown state of spi flash isn't partially (as noted in the FIXME). Therefore just drop the call and allow `emergency_help_message()` to be static local. Change-Id: Ib0d390c44e7a851e684014925a25c8b259b810cd Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M cli_classic.c M flashrom.c M include/flash.h 3 files changed, 22 insertions(+), 11 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/74/67474/4 -- To view, visit https://review.coreboot.org/c/flashrom/+/67474 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ib0d390c44e7a851e684014925a25c8b259b810cd Gerrit-Change-Number: 67474 Gerrit-PatchSet: 4 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: 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: Nico Huber <nico.h(a)gmx.de> 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-MessageType: newpatchset

1 0

[S] Change in flashrom[master]: flashrom.c: Confine is_internal checks under a symbol
by Edward O'Callaghan (Code Review) 11 Oct '22

11 Oct '22

Attention is currently required from: Felix Singer, Thomas Heijligen, Edward O'Callaghan, Anastasia Klimchuk. Hello Felix Singer, build bot (Jenkins), Thomas Heijligen, Angel Pons, Anastasia Klimchuk, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/67475 to look at the new patch set (#3). Change subject: flashrom.c: Confine is_internal checks under a symbol ...................................................................... flashrom.c: Confine is_internal checks under a symbol We wish for printers to be parametric on the flag state of if we are internal or not and not embed this side-effect. Change-Id: I569811798bfe310c59b8b61afba359bef68969fb Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M flashrom.c 1 file changed, 21 insertions(+), 8 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/75/67475/3 -- To view, visit https://review.coreboot.org/c/flashrom/+/67475 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I569811798bfe310c59b8b61afba359bef68969fb Gerrit-Change-Number: 67475 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: 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-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

[S] Change in flashrom[master]: flashrom.c: Drop emergency_help_message() static local
by Edward O'Callaghan (Code Review) 11 Oct '22

11 Oct '22

Attention is currently required from: Felix Singer, Nico Huber, Thomas Heijligen, Angel Pons. Hello Felix Singer, build bot (Jenkins), Thomas Heijligen, Angel Pons, Anastasia Klimchuk, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/67474 to look at the new patch set (#3). Change subject: flashrom.c: Drop emergency_help_message() static local ...................................................................... flashrom.c: Drop emergency_help_message() static local The emergency help message is only relevent under one specific sitution: i ) The internal programmer was used, && ii) The user was intending not to do a destructive operation. Since erasing the chip is inherantly a destructive operation that was requested warning the user they have a unknown state of spi flash isn't partially (as noted in the FIXME). Therefore just drop the call and allow `emergency_help_message()` to be static local. Change-Id: Ib0d390c44e7a851e684014925a25c8b259b810cd Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M cli_classic.c M flashrom.c M include/flash.h 3 files changed, 22 insertions(+), 11 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/74/67474/3 -- To view, visit https://review.coreboot.org/c/flashrom/+/67474 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ib0d390c44e7a851e684014925a25c8b259b810cd Gerrit-Change-Number: 67474 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: 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: Nico Huber <nico.h(a)gmx.de> 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-MessageType: newpatchset

1 0

[S] Change in flashrom[master]: flashrom.c: Make emergency_help_message() static local
by Edward O'Callaghan (Code Review) 11 Oct '22

11 Oct '22

Attention is currently required from: Felix Singer, Nico Huber, Thomas Heijligen, Angel Pons. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67474 ) Change subject: flashrom.c: Make emergency_help_message() static local ...................................................................... Patch Set 2: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/67474/comment/ad365f46_7724e76b PS2, Line 10: agnostic > That was my thought as well. We could move all the *_help_message() functions into […] The principle idea suggested here is not at all unreasonable and had crossed my mind already however in practical terms is hard to archive due to the lack of coupling between the programmer entry structure and the subsequent use of flashctx in the actual control flow graph. Since this specific call site of `emergency_help_message()` is the only thing in the way of pealing back the programmer global handle and it is after an erasure anyway maybe the better thing to do here is to just fix the `FIXME` and delete the call site to have the symbol static. Re-designing the logging framework is somewhat orthogonal to the task of getting a handle on the the `programmer_entry` singleton pattern in `flashrom.c`. I will push a revised pivot here towards that goal, WDYT? -- To view, visit https://review.coreboot.org/c/flashrom/+/67474 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ib0d390c44e7a851e684014925a25c8b259b810cd Gerrit-Change-Number: 67474 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: 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: Nico Huber <nico.h(a)gmx.de> 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-Comment-Date: Tue, 11 Oct 2022 00:03:28 +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

[M] Change in flashrom[master]: tree/: Convert flashchip decode range func ptr to enum
by Nikolai Artemiev (Code Review) 10 Oct '22

10 Oct '22

Attention is currently required from: Felix Singer, Anastasia Klimchuk. Hello Felix Singer, build bot (Jenkins), Edward O'Callaghan, Anastasia Klimchuk, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/67195 to look at the new patch set (#12). Change subject: tree/: Convert flashchip decode range func ptr to enum ...................................................................... tree/: Convert flashchip decode range func ptr to enum Replace the `decode_range` function pointer in `struct flashchip` to an enum value. The enum value can be used to find the corresponding function pointer by passing it to `lookup_decode_range_func_ptr()`. Removing function pointers like `decode_range` makes it possible to represent chip data in a declarative format that does not have to be stored as C source code. BUG=b:242479049 BRANCH=none TEST=ninja && ninja test Signed-off-by: Nikolai Artemiev <nartemiev(a)google.com> Change-Id: If6d08d414d3d1ddadc95ca1d407fc87c23ab543d --- M flashchips.c M include/flash.h M tests/chip_wp.c M writeprotect.c 4 files changed, 72 insertions(+), 28 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/95/67195/12 -- To view, visit https://review.coreboot.org/c/flashrom/+/67195 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: If6d08d414d3d1ddadc95ca1d407fc87c23ab543d Gerrit-Change-Number: 67195 Gerrit-PatchSet: 12 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

[M] Change in flashrom[master]: tree/: Convert flashchip decode range func ptr to enum
by Nikolai Artemiev (Code Review) 10 Oct '22

10 Oct '22

Attention is currently required from: Felix Singer, Anastasia Klimchuk. Hello Felix Singer, build bot (Jenkins), Edward O'Callaghan, Anastasia Klimchuk, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/67195 to look at the new patch set (#11). Change subject: tree/: Convert flashchip decode range func ptr to enum ...................................................................... tree/: Convert flashchip decode range func ptr to enum Replace the `decode_range` function pointer in `struct flashchip` to an enum value. The enum value can be used to find the corresponding function pointer by passing it to `lookup_decode_range_func_ptr()`. Removing function pointers from flash chip data will make it possible to represent the chip database in other formats that do not have to be compiled together with the rest of flashrom's source files. BUG=b:242479049 BRANCH=none TEST=ninja && ninja test Signed-off-by: Nikolai Artemiev <nartemiev(a)google.com> Change-Id: If6d08d414d3d1ddadc95ca1d407fc87c23ab543d --- M flashchips.c M include/flash.h M tests/chip_wp.c M writeprotect.c 4 files changed, 73 insertions(+), 28 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/95/67195/11 -- To view, visit https://review.coreboot.org/c/flashrom/+/67195 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: If6d08d414d3d1ddadc95ca1d407fc87c23ab543d Gerrit-Change-Number: 67195 Gerrit-PatchSet: 11 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

[M] Change in flashrom[master]: tree/: Convert flashchip decode range func ptr to enum
by Nikolai Artemiev (Code Review) 10 Oct '22

10 Oct '22

Attention is currently required from: Felix Singer, Anastasia Klimchuk. Nikolai Artemiev has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67195 ) Change subject: tree/: Convert flashchip decode range func ptr to enum ...................................................................... Patch Set 10: (3 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/67195/comment/33e05b89_ba4b7115 PS8, Line 10: to > Duplicate "to" Done https://review.coreboot.org/c/flashrom/+/67195/comment/2e38582a_fb4ae673 PS8, Line 12: This follows from on from previous patches that > This sounds confusing Done https://review.coreboot.org/c/flashrom/+/67195/comment/b688d8ba_29d0a372 PS8, Line 16: b/242479049 > You mean b:242479049 probably? Done -- To view, visit https://review.coreboot.org/c/flashrom/+/67195 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: If6d08d414d3d1ddadc95ca1d407fc87c23ab543d Gerrit-Change-Number: 67195 Gerrit-PatchSet: 10 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Mon, 10 Oct 2022 23:41:45 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Singer <felixsinger(a)posteo.net> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: flash.h: extend `struct tested` with .wp field
by Anastasia Klimchuk (Code Review) 10 Oct '22

10 Oct '22

Attention is currently required from: Nico Huber, Edward O'Callaghan, Anastasia Klimchuk, Sergii Dmytruk. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/68179 ) Change subject: flash.h: extend `struct tested` with .wp field ...................................................................... Patch Set 1: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/68179/comment/793ccc74_52575441 PS1, Line 9: "W" and "P" letters were already taken, so using "X" to represent : protection in macros. One can think of execution permission on Unix-like : systems, that also prevents running commands. > Oh wait... […] Maybe I confused myself, not sure. The meaningful chars count is exactly 72 ! It does show for some reasons a trailing space when I select the text. And that space overflows :) If you can check and remove the space, that would be it, thank you! -- To view, visit https://review.coreboot.org/c/flashrom/+/68179 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I791400889159bc6f305fb05f3e2dd9a90dbe18a4 Gerrit-Change-Number: 68179 Gerrit-PatchSet: 1 Gerrit-Owner: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.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: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.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-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Comment-Date: Mon, 10 Oct 2022 23:40:33 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit October 2022