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

Change in flashrom[master]: ene_lpc.c: Extract params check into a function
by Anastasia Klimchuk (Code Review) 28 Apr '21

28 Apr '21

Attention is currently required from: Nico Huber, Angel Pons. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52684 ) Change subject: ene_lpc.c: Extract params check into a function ...................................................................... Patch Set 2: (1 comment) File ene_lpc.c: https://review.coreboot.org/c/flashrom/+/52684/comment/7760832a_9f1db6b0 PS1, Line 519: = NULL > heh, I knew I missed something :) it's not a nit IMHO, static analysis […] Done, and also in mec1308 is the same check_params() function: https://review.coreboot.org/c/flashrom/+/52719 -- To view, visit https://review.coreboot.org/c/flashrom/+/52684 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7c3b6dea0edbc7547f0b307a0508c7d2b2a6d370 Gerrit-Change-Number: 52684 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: Miklós Márton <martonmiklosqdev(a)gmail.com> Gerrit-CC: Victor Ding <victording(a)google.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Wed, 28 Apr 2021 02:35:54 +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]: mec1308.c: Ensure programmer param variable is always initialised
by Anastasia Klimchuk (Code Review) 28 Apr '21

28 Apr '21

Anastasia Klimchuk has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/52719 ) Change subject: mec1308.c: Ensure programmer param variable is always initialised ...................................................................... mec1308.c: Ensure programmer param variable is always initialised Programmer param variable is now declared inline with the code and is const, so that it cannot be used uninitialised. BUG=b:185191942 TEST=builds and ninja test from 51487 Change-Id: I2d2b3039da2ef185cb31509b3901d56b428688b7 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M mec1308.c 1 file changed, 1 insertion(+), 3 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/19/52719/1 diff --git a/mec1308.c b/mec1308.c index 0ec9be9..1530d9b 100644 --- a/mec1308.c +++ b/mec1308.c @@ -409,9 +409,7 @@ static int check_params(void) { int ret = 0; - char *p = NULL; - - p = extract_programmer_param("type"); + char *const p = extract_programmer_param("type"); if (p && strcmp(p, "ec")) { msg_pdbg("mec1308 only supports \"ec\" type devices\n"); ret = 1; -- To view, visit https://review.coreboot.org/c/flashrom/+/52719 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2d2b3039da2ef185cb31509b3901d56b428688b7 Gerrit-Change-Number: 52719 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: flashrom.c: allow - as filename for stdin
by Edward O'Callaghan (Code Review) 28 Apr '21

28 Apr '21

Attention is currently required from: Nico Huber, Daniel Campello, Angel Pons. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52383 ) Change subject: flashrom.c: allow - as filename for stdin ...................................................................... Patch Set 14: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/52383 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I97889cfdf7ba9a257e182c4ee2b20075cfa58d4d Gerrit-Change-Number: 52383 Gerrit-PatchSet: 14 Gerrit-Owner: Daniel Campello <campello(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Jack Rosenthal <jrosenth(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: Daniel Campello <campello(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Wed, 28 Apr 2021 01:32:36 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: flashrom.c: allow - as filename for stdin
by Daniel Campello (Code Review) 28 Apr '21

28 Apr '21

Attention is currently required from: Nico Huber, Daniel Campello, Angel Pons. Hello build bot (Jenkins), Nico Huber, Jack Rosenthal, Edward O'Callaghan, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/52383 to look at the new patch set (#14). Change subject: flashrom.c: allow - as filename for stdin ...................................................................... flashrom.c: allow - as filename for stdin Allows - as filename for -w/-v options. It is sometimes useful to script flashrom and allowing it to work with pipes allows for more flexibility in this specific use-case. Signed-off-by: Daniel Campello <campello(a)chromium.org> Change-Id: I97889cfdf7ba9a257e182c4ee2b20075cfa58d4d --- M cli_classic.c M flashrom.c 2 files changed, 11 insertions(+), 5 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/83/52383/14 -- To view, visit https://review.coreboot.org/c/flashrom/+/52383 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I97889cfdf7ba9a257e182c4ee2b20075cfa58d4d Gerrit-Change-Number: 52383 Gerrit-PatchSet: 14 Gerrit-Owner: Daniel Campello <campello(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Jack Rosenthal <jrosenth(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: Daniel Campello <campello(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: ene_lpc.c: Untangle successful vs failed init paths
by Anastasia Klimchuk (Code Review) 28 Apr '21

28 Apr '21

Attention is currently required from: Nico Huber. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52685 ) Change subject: ene_lpc.c: Untangle successful vs failed init paths ...................................................................... Patch Set 3: (1 comment) File ene_lpc.c: https://review.coreboot.org/c/flashrom/+/52685/comment/d357ad47_26e7c492 PS1, Line 575: ene_leave_flash_mode > NB. should be called on the error path. I introduced second label here (same as in mec1308) which does cleanup. I will be using the same label names where this pattern is needed. -- To view, visit https://review.coreboot.org/c/flashrom/+/52685 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iac295f1353785cd73d7cb2f19e4a8cbb69beb576 Gerrit-Change-Number: 52685 Gerrit-PatchSet: 3 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: Miklós Márton <martonmiklosqdev(a)gmail.com> Gerrit-CC: Victor Ding <victording(a)google.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Comment-Date: Wed, 28 Apr 2021 01:09:09 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ene_lpc.c: Extract params check into a function
by Anastasia Klimchuk (Code Review) 28 Apr '21

28 Apr '21

Anastasia Klimchuk has abandoned this change. ( https://review.coreboot.org/c/flashrom/+/52718 ) Change subject: ene_lpc.c: Extract params check into a function ...................................................................... Abandoned This is created by mistake, a duplicate of 52684 -- To view, visit https://review.coreboot.org/c/flashrom/+/52718 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7cf99d10a75ad4ace4216fcc5719c580be84389c Gerrit-Change-Number: 52718 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: abandon

1 0

Change in flashrom[master]: ene_lpc.c: Untangle successful vs failed init paths
by Anastasia Klimchuk (Code Review) 28 Apr '21

28 Apr '21

Attention is currently required from: 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/+/52685 to look at the new patch set (#3). Change subject: ene_lpc.c: Untangle successful vs failed init paths ...................................................................... ene_lpc.c: Untangle successful vs failed init paths Exit label now serves as failed init path, it does cleanup and returns 1, so it is renamed into init_err_exit. Since all error paths return 1, and successful init is separated from failure, there is no need to have ret variable anymore. BUG=b:185191942 TEST=builds Change-Id: Iac295f1353785cd73d7cb2f19e4a8cbb69beb576 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M ene_lpc.c 1 file changed, 14 insertions(+), 15 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/85/52685/3 -- To view, visit https://review.coreboot.org/c/flashrom/+/52685 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iac295f1353785cd73d7cb2f19e4a8cbb69beb576 Gerrit-Change-Number: 52685 Gerrit-PatchSet: 3 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: Miklós Márton <martonmiklosqdev(a)gmail.com> Gerrit-CC: Victor Ding <victording(a)google.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: ene_lpc.c: Extract params check into a function
by Anastasia Klimchuk (Code Review) 28 Apr '21

28 Apr '21

Attention is currently required from: 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/+/52684 to look at the new patch set (#2). Change subject: ene_lpc.c: Extract params check into a function ...................................................................... ene_lpc.c: Extract params check into a function This allows char *p to become a local variable in check_params, and it is allocated and freed within check_params function. Which means init function does not need char *p anymore, in particular does not need to free it - and this makes cleanup after failed init easier. As a good side effect, init function becomes easier to read. BUG=b:185191942 TEST=builds Change-Id: I7c3b6dea0edbc7547f0b307a0508c7d2b2a6d370 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M ene_lpc.c 1 file changed, 14 insertions(+), 5 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/84/52684/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/52684 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7c3b6dea0edbc7547f0b307a0508c7d2b2a6d370 Gerrit-Change-Number: 52684 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: Miklós Márton <martonmiklosqdev(a)gmail.com> Gerrit-CC: Victor Ding <victording(a)google.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: ene_lpc.c: Extract params check into a function
by Anastasia Klimchuk (Code Review) 28 Apr '21

28 Apr '21

Anastasia Klimchuk has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/52718 ) Change subject: ene_lpc.c: Extract params check into a function ...................................................................... ene_lpc.c: Extract params check into a function This allows char *p to become a local variable in check_params, and it is allocated and freed within check_params function. Which means init function does not need char *p anymore, in particular does not need to free it - and this makes cleanup after failed init easier. As a good side effect, init function becomes easier to read. BUG=b:185191942 TEST=builds Change-Id: I7cf99d10a75ad4ace4216fcc5719c580be84389c Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M ene_lpc.c 1 file changed, 14 insertions(+), 5 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/18/52718/1 diff --git a/ene_lpc.c b/ene_lpc.c index 1d045b8..c05fef8 100644 --- a/ene_lpc.c +++ b/ene_lpc.c @@ -513,11 +513,23 @@ .write_256 = default_spi_write_256, }; +static int check_params(void) +{ + int ret = 0; + char *const p = extract_programmer_param("type"); + if (p && strcmp(p, "ec")) { + msg_pdbg("ene_lpc only supports \"ec\" type devices\n"); + ret = 1; + } + + free(p); + return ret; +} + int ene_lpc_init() { uint8_t hwver, ediid, i; int ret = 0; - char *p = NULL; ene_lpc_data_t *ctx_data = NULL; msg_pdbg("%s\n", __func__); @@ -529,9 +541,7 @@ } ctx_data->ec_state = EC_STATE_NORMAL; - p = extract_programmer_param("type"); - if (p && strcmp(p, "ec")) { - msg_pdbg("ene_lpc only supports \"ec\" type devices\n"); + if (check_params()) { ret = 1; goto ene_probe_spi_flash_exit; } @@ -572,7 +582,6 @@ msg_pdbg("%s: successfully initialized ene\n", __func__); ene_probe_spi_flash_exit: - free(p); if (ret) free(ctx_data); return ret; -- To view, visit https://review.coreboot.org/c/flashrom/+/52718 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7cf99d10a75ad4ace4216fcc5719c580be84389c Gerrit-Change-Number: 52718 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: ene_lpc.c: Untangle successful vs failed init paths
by Anastasia Klimchuk (Code Review) 28 Apr '21

28 Apr '21

Attention is currently required from: 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/+/52685 to look at the new patch set (#2). Change subject: ene_lpc.c: Untangle successful vs failed init paths ...................................................................... ene_lpc.c: Untangle successful vs failed init paths Exit label now serves as failed init path, it does cleanup and returns 1, so it is renamed into init_err_exit. Since all error paths return 1, and successful init is separated from failure, there is no need to have ret variable anymore. BUG=b:185191942 TEST=builds Change-Id: Iac295f1353785cd73d7cb2f19e4a8cbb69beb576 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M ene_lpc.c 1 file changed, 14 insertions(+), 15 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/85/52685/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/52685 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iac295f1353785cd73d7cb2f19e4a8cbb69beb576 Gerrit-Change-Number: 52685 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: Miklós Márton <martonmiklosqdev(a)gmail.com> Gerrit-CC: Victor Ding <victording(a)google.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit April 2021