March 2023 - flashrom-gerrit - mail.coreboot.org

[M] Change in flashrom[master]: cli_classic: move validation of incompatible options to one place
by Alexander Goncharov (Code Review) 02 Mar '23

02 Mar '23

Attention is currently required from: Felix Singer, Alexander Goncharov. Hello Felix Singer, build bot (Jenkins), Edward O'Callaghan, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/66532 to look at the new patch set (#5). Change subject: cli_classic: move validation of incompatible options to one place ...................................................................... cli_classic: move validation of incompatible options to one place Avoid repeating the same messages. TOPIC=split_main_func TEST=builds Change-Id: If6d7579847cc8ae13b54ef7bd50072a9402f835f Signed-off-by: Alexander Goncharov <chat(a)joursoir.net> --- M cli_classic.c 1 file changed, 27 insertions(+), 33 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/32/66532/5 -- To view, visit https://review.coreboot.org/c/flashrom/+/66532 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: If6d7579847cc8ae13b54ef7bd50072a9402f835f Gerrit-Change-Number: 66532 Gerrit-PatchSet: 5 Gerrit-Owner: Alexander Goncharov <chat(a)joursoir.net> 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: Alexander Goncharov <chat(a)joursoir.net> Gerrit-MessageType: newpatchset

1 0

[M] Change in flashrom[master]: cli_classic: move validation of incompatible options to one place
by Alexander Goncharov (Code Review) 02 Mar '23

02 Mar '23

Attention is currently required from: Felix Singer, Alexander Goncharov. Hello Felix Singer, build bot (Jenkins), Edward O'Callaghan, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/66532 to look at the new patch set (#4). Change subject: cli_classic: move validation of incompatible options to one place ...................................................................... cli_classic: move validation of incompatible options to one place Avoid repeating the same messages. TOPIC=split_main_func TEST=builds Change-Id: If6d7579847cc8ae13b54ef7bd50072a9402f835f Signed-off-by: Alexander Goncharov <chat(a)joursoir.net> --- M cli_classic.c 1 file changed, 29 insertions(+), 33 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/32/66532/4 -- To view, visit https://review.coreboot.org/c/flashrom/+/66532 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: If6d7579847cc8ae13b54ef7bd50072a9402f835f Gerrit-Change-Number: 66532 Gerrit-PatchSet: 4 Gerrit-Owner: Alexander Goncharov <chat(a)joursoir.net> 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: Alexander Goncharov <chat(a)joursoir.net> Gerrit-MessageType: newpatchset

1 0

[M] Change in flashrom[master]: cli_classic: wrap options validation in function
by Alexander Goncharov (Code Review) 02 Mar '23

02 Mar '23

Attention is currently required from: Felix Singer, Alexander Goncharov. Hello Felix Singer, build bot (Jenkins), Edward O'Callaghan, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/66531 to look at the new patch set (#4). Change subject: cli_classic: wrap options validation in function ...................................................................... cli_classic: wrap options validation in function Introduce a function that will be responsible for checking the parameters. This is one of the steps on the way to simplify the main function by using helper functions with descriptive names. TOPIC=split_main_func TEST=builds Change-Id: Ib5685eb5ee3810ef2efd40a3901340e2ceb229ff Signed-off-by: Alexander Goncharov <chat(a)joursoir.net> --- M cli_classic.c 1 file changed, 37 insertions(+), 13 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/31/66531/4 -- To view, visit https://review.coreboot.org/c/flashrom/+/66531 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ib5685eb5ee3810ef2efd40a3901340e2ceb229ff Gerrit-Change-Number: 66531 Gerrit-PatchSet: 4 Gerrit-Owner: Alexander Goncharov <chat(a)joursoir.net> 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: Alexander Goncharov <chat(a)joursoir.net> Gerrit-MessageType: newpatchset

1 0

[L] Change in flashrom[master]: cli_classic: refactor arguments parsing into separate func
by Alexander Goncharov (Code Review) 02 Mar '23

02 Mar '23

Attention is currently required from: Felix Singer, Edward O'Callaghan. Hello Felix Singer, build bot (Jenkins), Edward O'Callaghan, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/66343 to look at the new patch set (#7). Change subject: cli_classic: refactor arguments parsing into separate func ...................................................................... cli_classic: refactor arguments parsing into separate func Move variables that represent parsed options to `cli_options` structure. This patchset also introduces the following functions: - parse_options() which parses command line arguments and fills the structure. - free_options() that releases an allocated data from the structure. This is one of the steps on the way to simplify the main function by using helper functions with descriptive names. TOPIC=split_main_func TEST=builds Change-Id: Id573bc74e3bb46b7dc42f1452fff6394d4f091bc Signed-off-by: Alexander Goncharov <chat(a)joursoir.net> --- M cli_classic.c 1 file changed, 388 insertions(+), 343 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/43/66343/7 -- To view, visit https://review.coreboot.org/c/flashrom/+/66343 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id573bc74e3bb46b7dc42f1452fff6394d4f091bc Gerrit-Change-Number: 66343 Gerrit-PatchSet: 7 Gerrit-Owner: Alexander Goncharov <chat(a)joursoir.net> 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: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

[S] Change in flashrom[master]: meson: revert to meson version 0.53.0
by Anastasia Klimchuk (Code Review) 02 Mar '23

02 Mar '23

Attention is currently required from: Thomas Heijligen, Edward O'Callaghan, Alexander Goncharov. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/73361 ) Change subject: meson: revert to meson version 0.53.0 ...................................................................... Patch Set 2: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/73361/comment/1e9dd381_cca7bd2f PS1, Line 10: Pass arguments to `sphinx-doc` via `'-Dkey=value` > In `doc/meson.build` where sphinx is called as custom_target. […] Yes, I understand it better now, thanks for the explanation! I think it would be very good to add the specific info into commit message: instead of saying "key=value" say what option is set and why. Pass release version into sphinx-doc because it is needed for XYZ reasons -- To view, visit https://review.coreboot.org/c/flashrom/+/73361 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iff9b8307c741a247a652cf666935c9485fa493fa Gerrit-Change-Number: 73361 Gerrit-PatchSet: 2 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> 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-Attention: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Comment-Date: Thu, 02 Mar 2023 11:50:12 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Thomas Heijligen <src(a)posteo.de> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: flashrom: rewrite flashbuses_to_text()
by Anastasia Klimchuk (Code Review) 02 Mar '23

02 Mar '23

Attention is currently required from: Felix Singer, Edward O'Callaghan, Angel Pons, Alexander Goncharov. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/73039 ) Change subject: flashrom: rewrite flashbuses_to_text() ...................................................................... Patch Set 5: (1 comment) File flashrom.c: https://review.coreboot.org/c/flashrom/+/73039/comment/cb7a8dbe_733da77a PS4, Line 924: char *ret, *ptr; > Hmm... `strcat_realloc()` may return NULL. In such case `ret = strcat_realloc(ret, ... […] Done -- To view, visit https://review.coreboot.org/c/flashrom/+/73039 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I59b9044c99b4ba6c00d8c97f1e91af09d70dce2c Gerrit-Change-Number: 73039 Gerrit-PatchSet: 5 Gerrit-Owner: Alexander Goncharov <chat(a)joursoir.net> 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Comment-Date: Thu, 02 Mar 2023 11:39:48 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Comment-In-Reply-To: Alexander Goncharov <chat(a)joursoir.net> Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: flashrom: rewrite flashbuses_to_text()
by Anastasia Klimchuk (Code Review) 02 Mar '23

02 Mar '23

Attention is currently required from: Felix Singer, Edward O'Callaghan, Angel Pons, Alexander Goncharov. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/73039 ) Change subject: flashrom: rewrite flashbuses_to_text() ...................................................................... Patch Set 5: (1 comment) File flashrom.c: https://review.coreboot.org/c/flashrom/+/73039/comment/9885d239_3f8f3b94 PS5, Line 948: if (bustype & (1 << i)) why this condition is needed? -- To view, visit https://review.coreboot.org/c/flashrom/+/73039 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I59b9044c99b4ba6c00d8c97f1e91af09d70dce2c Gerrit-Change-Number: 73039 Gerrit-PatchSet: 5 Gerrit-Owner: Alexander Goncharov <chat(a)joursoir.net> 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Comment-Date: Thu, 02 Mar 2023 11:38:48 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: meson: revert to meson version 0.53.0
by Thomas Heijligen (Code Review) 02 Mar '23

02 Mar '23

Attention is currently required from: Edward O'Callaghan, Anastasia Klimchuk, Alexander Goncharov. Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/73361 ) Change subject: meson: revert to meson version 0.53.0 ...................................................................... Patch Set 2: (2 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/73361/comment/da546866_923be864 PS1, Line 9: 0.63.2 > Typo. https://packages.debian.org/bullseye/meson says about meson 0.56. […] Thanks for spotting. https://review.coreboot.org/c/flashrom/+/73361/comment/726b5c92_98124394 PS1, Line 10: Pass arguments to `sphinx-doc` via `'-Dkey=value` > I feel that I am missing something, but where this is in the patch? In `doc/meson.build` where sphinx is called as custom_target. Originally I used `FLASHROM_VERSION=meson.project_version()` in the environment of the custom_target and `os.getenv('FLASHROM_VERSION')` in `doc/conf.py` (the configuration of sphinx) to pass the version information. Now I pass `-Drelease=meson.project_version()` as parameter of the `sphinx` call, which directly writes the `release` variable in sphinx. From `sphinx-build`: ``` -D setting=value override a setting in configuration file ``` Is it now better to understand? Should I put this comment also in the commit message? -- To view, visit https://review.coreboot.org/c/flashrom/+/73361 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iff9b8307c741a247a652cf666935c9485fa493fa Gerrit-Change-Number: 73361 Gerrit-PatchSet: 2 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Comment-Date: Thu, 02 Mar 2023 11:37:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Comment-In-Reply-To: Alexander Goncharov <chat(a)joursoir.net> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: meson: revert to meson version 0.53.0
by Thomas Heijligen (Code Review) 02 Mar '23

02 Mar '23

Attention is currently required from: Thomas Heijligen, Edward O'Callaghan. Hello build bot (Jenkins), Edward O'Callaghan, Anastasia Klimchuk, Alexander Goncharov, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/73361 to look at the new patch set (#2). Change subject: meson: revert to meson version 0.53.0 ...................................................................... meson: revert to meson version 0.53.0 Debian Bullseye, current stable release, ships only with meson 0.56.2. Pass arguments to `sphinx-doc` via `'-Dkey=value` instead of environment variables to stay compatible with older meson version. Change-Id: Iff9b8307c741a247a652cf666935c9485fa493fa Signed-off-by: Thomas Heijligen <thomas.heijligen(a)secunet.com> --- M doc/conf.py M doc/meson.build M meson.build 3 files changed, 17 insertions(+), 6 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/61/73361/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/73361 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iff9b8307c741a247a652cf666935c9485fa493fa Gerrit-Change-Number: 73361 Gerrit-PatchSet: 2 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> 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-MessageType: newpatchset

1 0

[M] Change in flashrom[master]: flashrom: rewrite flashbuses_to_text()
by Alexander Goncharov (Code Review) 02 Mar '23

02 Mar '23

Attention is currently required from: Felix Singer, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk. Alexander Goncharov has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/73039 ) Change subject: flashrom: rewrite flashbuses_to_text() ...................................................................... Patch Set 5: (3 comments) File flashrom.c: https://review.coreboot.org/c/flashrom/+/73039/comment/af0ed7f2_7316e0a5 PS4, Line 918: Get > You can leave "return" in this case, I think return sounds good here. Done https://review.coreboot.org/c/flashrom/+/73039/comment/0ceb87d6_6b4fcc95 PS4, Line 924: char *ret, *ptr; > I am going to ask silly question: why do you need both? […] Hmm... `strcat_realloc()` may return NULL. In such case `ret = strcat_realloc(ret, ...);` we will lose the pointer to the allocated space => memory leak https://review.coreboot.org/c/flashrom/+/73039/comment/df5da46d_f8e86b9f PS4, Line 938: if (bustype == BUS_NONSPI) : return strdup("Non-SPI"); : if (bustype == BUS_NONE) : return strdup(bustypes[bustype]); > These first two cases (BUS_NONSPI and BUS_NONE) are specially handled in the code, I understand why, […] You're definitely right, your suggestion will simplify everything, thanks! Done. -- To view, visit https://review.coreboot.org/c/flashrom/+/73039 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I59b9044c99b4ba6c00d8c97f1e91af09d70dce2c Gerrit-Change-Number: 73039 Gerrit-PatchSet: 5 Gerrit-Owner: Alexander Goncharov <chat(a)joursoir.net> 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Thu, 02 Mar 2023 10:39:14 +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 March 2023