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

Change in flashrom[master]: [WIP] replace programmer enum with a pointer to programmer_entry
by Nico Huber (Code Review) 31 Mar '21

31 Mar '21

Attention is currently required from: Thomas Heijligen. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/51925 ) Change subject: [WIP] replace programmer enum with a pointer to programmer_entry ...................................................................... Patch Set 4: (5 comments) This change is ready for review. Patchset: PS4: Looks quite good already. I think the DEFAULT_PROGRAMMER issue (and also the INTERNAL_PROGRAMMER checks) would solve itself if you would move the programmer_entry structs to the individual source files, first. e.g. * first commit: move programmer_entry, turn programmer_table into an array of pointers (leaving the enum stuff as is for now), and s/programmer_table[.*]\./programmer_table[.*]->/ everywhere. * second commit: introduce a default_programmer pointer. (const, set at compile time, replacing CONFIG_DEFAULT_PROGRAMMER). * third commit: this change, using `default_programmer` and `&internal_programmer` where needed. File cli_classic.c: https://review.coreboot.org/c/flashrom/+/51925/comment/de8b970a_e4e10051 PS4, Line 379: size_t _i Common flashrom style is to avoid such `for` local declarations. What I usually say here: the style should be discussed on the mailing list (and I really wouldn't mind). Anyway, you can just use `i` here. https://review.coreboot.org/c/flashrom/+/51925/comment/5e7672d3_809b9a64 PS4, Line 383: prog = &programmer_table[_i]; This needs to be moved to the positive cases below (':' and '\0'). Otherwise we would turn this into a prefix match by accident. Or, when we hit the `continue;` below, it should be reset to `NULL`. File flashrom.c: https://review.coreboot.org/c/flashrom/+/51925/comment/f6a76ee5_276acf11 PS4, Line 2452: cb_check_image(newcontents, flash_size) < 0) { Should be indented with either 4 spaces or 2 tabs (shouldn't match with the body). Or don't break the line: our limit is 112 chars[1]. [1] https://flashrom.org/Development_Guidelines#Coding_style File libflashrom.c: https://review.coreboot.org/c/flashrom/+/51925/comment/5f85e505_4c9930cc PS4, Line 128: } This is quite weird. It leaves the last two pointers uninitialized. The API seems also meaningless, there is no way to know how many items are returned. -- To view, visit https://review.coreboot.org/c/flashrom/+/51925 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7435ee2bb661e4f139034bb289b84ebd41e1e9e9 Gerrit-Change-Number: 51925 Gerrit-PatchSet: 4 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Xiang Wang <merle(a)hardenedlinux.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Comment-Date: Wed, 31 Mar 2021 14:33:30 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: test_build.sh: use -C option of ninja to specify the build directory
by Thomas Heijligen (Code Review) 31 Mar '21

31 Mar '21

Attention is currently required from: Nico Huber, Paul Menzel, Angel Pons. Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/51979 ) Change subject: test_build.sh: use -C option of ninja to specify the build directory ...................................................................... Patch Set 3: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/51979/comment/4412a0ec_685d30ba PS2, Line 7: test_build.sh: use -C option of ninja > … to specify the build directory Done -- To view, visit https://review.coreboot.org/c/flashrom/+/51979 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I04a0fdf9b5126b9f4006e8229c3926ceb1013456 Gerrit-Change-Number: 51979 Gerrit-PatchSet: 3 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Wed, 31 Mar 2021 13:24:52 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: test_build.sh: use -C option of ninja to specify the build directory
by Thomas Heijligen (Code Review) 31 Mar '21

31 Mar '21

Attention is currently required from: Nico Huber, Thomas Heijligen, Angel Pons. Hello build bot (Jenkins), Nico Huber, Patrick Georgi, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/51979 to look at the new patch set (#3). Change subject: test_build.sh: use -C option of ninja to specify the build directory ...................................................................... test_build.sh: use -C option of ninja to specify the build directory Change-Id: I04a0fdf9b5126b9f4006e8229c3926ceb1013456 Signed-off-by: Thomas Heijligen <thomas.heijligen(a)secunet.de> --- M test_build.sh 1 file changed, 5 insertions(+), 3 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/79/51979/3 -- To view, visit https://review.coreboot.org/c/flashrom/+/51979 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I04a0fdf9b5126b9f4006e8229c3926ceb1013456 Gerrit-Change-Number: 51979 Gerrit-PatchSet: 3 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.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

Change in flashrom[master]: test_build.sh: use sh from env
by Angel Pons (Code Review) 31 Mar '21

31 Mar '21

Attention is currently required from: Nico Huber, Paul Menzel, Thomas Heijligen. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/51978 ) Change subject: test_build.sh: use sh from env ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: > I think there are pros and cons for this pattern. Which ones? I'm curious. If I were an English teacher, I'd ask you to write a for-and-against essay. 😜 But since I'm not, a brief explanation is more than enough 😊 -- To view, visit https://review.coreboot.org/c/flashrom/+/51978 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3897b8d980425ecbb89b238d4a766f628cf9d3e6 Gerrit-Change-Number: 51978 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Comment-Date: Wed, 31 Mar 2021 12:44:07 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: test_build.sh: use sh from env
by Paul Menzel (Code Review) 31 Mar '21

31 Mar '21

Attention is currently required from: Nico Huber, Thomas Heijligen. Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/51978 ) Change subject: test_build.sh: use sh from env ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: I think there are pros and cons for this pattern. -- To view, visit https://review.coreboot.org/c/flashrom/+/51978 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3897b8d980425ecbb89b238d4a766f628cf9d3e6 Gerrit-Change-Number: 51978 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Comment-Date: Wed, 31 Mar 2021 12:35:52 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: test_build.sh: use sh from env
by Paul Menzel (Code Review) 31 Mar '21

31 Mar '21

Attention is currently required from: Nico Huber, Thomas Heijligen. Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/51978 ) Change subject: test_build.sh: use sh from env ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/51978 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3897b8d980425ecbb89b238d4a766f628cf9d3e6 Gerrit-Change-Number: 51978 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Comment-Date: Wed, 31 Mar 2021 12:35:41 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: test_build.sh: use -C option of ninja
by Paul Menzel (Code Review) 31 Mar '21

31 Mar '21

Attention is currently required from: Nico Huber, Thomas Heijligen, Angel Pons. Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/51979 ) Change subject: test_build.sh: use -C option of ninja ...................................................................... Patch Set 2: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/51979/comment/3fe3b02a_559e49a9 PS2, Line 7: test_build.sh: use -C option of ninja … to specify the build directory -- To view, visit https://review.coreboot.org/c/flashrom/+/51979 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I04a0fdf9b5126b9f4006e8229c3926ceb1013456 Gerrit-Change-Number: 51979 Gerrit-PatchSet: 2 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.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: Wed, 31 Mar 2021 12:35:29 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: test_build.sh: use -C option of ninja
by Thomas Heijligen (Code Review) 31 Mar '21

31 Mar '21

Attention is currently required from: Nico Huber, Angel Pons. Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/51979 ) Change subject: test_build.sh: use -C option of ninja ...................................................................... Patch Set 2: (1 comment) File test_build.sh: https://review.coreboot.org/c/flashrom/+/51979/comment/15906a39_9f4c7e0f PS1, Line 4: #make CONFIG_EVERYTHING=yes WARNERROR=yes > Did you mean to comment this line out? No, that was a mistake. I've not all dependencies for the make build available and commend it out for testing the meson part -- To view, visit https://review.coreboot.org/c/flashrom/+/51979 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I04a0fdf9b5126b9f4006e8229c3926ceb1013456 Gerrit-Change-Number: 51979 Gerrit-PatchSet: 2 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Wed, 31 Mar 2021 11:45:35 +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]: test_build.sh: use -C option of ninja
by Thomas Heijligen (Code Review) 31 Mar '21

31 Mar '21

Attention is currently required from: Nico Huber, Thomas Heijligen. Hello build bot (Jenkins), Nico Huber, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/51979 to look at the new patch set (#2). Change subject: test_build.sh: use -C option of ninja ...................................................................... test_build.sh: use -C option of ninja Change-Id: I04a0fdf9b5126b9f4006e8229c3926ceb1013456 Signed-off-by: Thomas Heijligen <thomas.heijligen(a)secunet.de> --- M test_build.sh 1 file changed, 5 insertions(+), 3 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/79/51979/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/51979 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I04a0fdf9b5126b9f4006e8229c3926ceb1013456 Gerrit-Change-Number: 51979 Gerrit-PatchSet: 2 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: test_build.sh: use -C option of ninja
by Angel Pons (Code Review) 31 Mar '21

31 Mar '21

Attention is currently required from: Nico Huber, Thomas Heijligen. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/51979 ) Change subject: test_build.sh: use -C option of ninja ...................................................................... Patch Set 1: (1 comment) File test_build.sh: https://review.coreboot.org/c/flashrom/+/51979/comment/d65d63ed_dfe9ff0a PS1, Line 4: #make CONFIG_EVERYTHING=yes WARNERROR=yes Did you mean to comment this line out? -- To view, visit https://review.coreboot.org/c/flashrom/+/51979 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I04a0fdf9b5126b9f4006e8229c3926ceb1013456 Gerrit-Change-Number: 51979 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Comment-Date: Wed, 31 Mar 2021 11:39:00 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit March 2021