September 2022 - flashrom-gerrit

[S] Change in flashrom[master]: ichspi.c: Retype appropiate variables with bool
by Felix Singer (Code Review) 07 Sep '22

07 Sep '22

Attention is currently required from: Nico Huber, Angel Pons. Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/66892 ) Change subject: ichspi.c: Retype appropiate variables with bool ...................................................................... Patch Set 10: (1 comment) File ichspi.c: https://review.coreboot.org/c/flashrom/+/66892/comment/3c96429e_b49b15ac PS7, Line 872: write_cmd = true; > IMHO, if this was a single "treewide: retype variables as bool where appropriate", I might consider […] Thanks Angel for your comment! You summarized the situation very good. To give you all some background about how I came to this patch series: I am reworking the programmer arguments parser and I am thinking about how we can deduplicate more code. Since the idea is to make things more type-aware and also all programmers do pretty much the same thing, many things can be centralized and deduplicated. In other words, the parsing and conversion part can be pretty much moved out of the programmer code to one place. However, it just helps a lot when I see the appropriate types at the variables, since this will be important later when I do the actual rework (hooking the new parser up). So I decided to go over the whole tree instead over just the programmers. I decided to make multiple commits because I think people wouldn't have been able to review the changes else. Unfortunately these changes are not reproducible. If so, I would have done one single patch. I rather would like to get this patch series in now and do the variable renaming in a separate patch(-series) later, because this series already has been reviewed by Anastasia and others now and it's ready. So it doesn't matter much if I create a dedicated patch series or update this one. At this point the work has to be done twice, since in both cases I have to (re-)write the commit messages (to mention the renaming) and people need to review everything (again). I think it's very reasonable to separate both changes. It's a very long patch series and this way reviewers can concentrate on one specific thing, which makes it more easier to review. The renaming changes could be done in a single patch since it should be reproducible. -- To view, visit https://review.coreboot.org/c/flashrom/+/66892 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: If7eeacc44921f52aa593ab1302f17a5c5190f830 Gerrit-Change-Number: 66892 Gerrit-PatchSet: 10 Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> 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, 07 Sep 2022 01:05:38 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Singer <felixsinger(a)posteo.net> Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> 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

[S] Change in flashrom[master]: tests: Add workaround to allow tests mock fileno on FreeBSD
by Anastasia Klimchuk (Code Review) 07 Sep '22

07 Sep '22

Attention is currently required from: Thomas Heijligen, Edward O'Callaghan, Peter Marheine. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67312 ) Change subject: tests: Add workaround to allow tests mock fileno on FreeBSD ...................................................................... Patch Set 4: (1 comment) File tests/chip.c: https://review.coreboot.org/c/flashrom/+/67312/comment/c57d0b21_6d5357a3 PS2, Line 161: __isthreaded = 0; > Or we could handle all libc calls like we already do for malloc/free. […] Okay there are three points here and at the moment I did one (in the latest PS), the other two I would like to clarify. 1) I did what Peter said: removed `__isthreaded = 0` from teardown() block. 2) > also perhaps it maybe a good idea to set it right at the very beginning of the whole test suite not 'half way' in when the control flow arrives at setup_chip() I was thinking about it, but there is only one place, which is literally at the beginning of the whole test suite: in tests/tests.c#main function. Did you mean that? 3) > we could handle all libc calls like we already do for malloc/free. Since all of them can be implemented as macros This is an interesting idea! Needs a research (before I can say Yes let's do it). Also I think if we would be handling all libc calls that would be much larger patch. But doing this can give benefits long-term. Can we have this patch now and try your larger idea later, what do you think? -- To view, visit https://review.coreboot.org/c/flashrom/+/67312 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3789ea9107a4cf8033cf59bb96d3c82aa58de026 Gerrit-Change-Number: 67312 Gerrit-PatchSet: 4 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-CC: Idwer Vollering <vidwer(a)gmail.com> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Wed, 07 Sep 2022 00:39:25 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Thomas Heijligen <src(a)posteo.de> Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Comment-In-Reply-To: Peter Marheine <pmarheine(a)chromium.org> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: tests: Guard Linux-specific linux/spidev.h header file
by Anastasia Klimchuk (Code Review) 07 Sep '22

07 Sep '22

Attention is currently required from: Thomas Heijligen. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67311 ) Change subject: tests: Guard Linux-specific linux/spidev.h header file ...................................................................... Patch Set 4: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/67311/comment/d426eaa1_ea8f2936 PS2, Line 14: FreeBSD 13.1-RELEASE-p2 GENERIC amd64 > Same as before, With only this one, tests are compiling but not working. You are right, I updated commit message so now it says "tests compile on two environments" -- To view, visit https://review.coreboot.org/c/flashrom/+/67311 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Icdb1b0cb29c5d62187153788ad5e0e631e8d0b62 Gerrit-Change-Number: 67311 Gerrit-PatchSet: 4 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Idwer Vollering <vidwer(a)gmail.com> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Comment-Date: Wed, 07 Sep 2022 00:09:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Thomas Heijligen <src(a)posteo.de> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: tests: Use MOCK_FD instead of NON_ZERO for file operations
by Anastasia Klimchuk (Code Review) 07 Sep '22

07 Sep '22

Attention is currently required from: Thomas Heijligen. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67310 ) Change subject: tests: Use MOCK_FD instead of NON_ZERO for file operations ...................................................................... Patch Set 4: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/67310/comment/d2f9e2c8_f3dde907 PS2, Line 15: FreeBSD 13.1-RELEASE-p2 GENERIC amd64 > The patch sets are in wrong order for this. […] My mistake was to copy-paste the same test info for all 3 patches, sorry. Patches can probably go in any order, but all of them needed to run the tests on freebsd, so tests can run at the end of the chain (not in the middle). I updated commit messages, hopefully they are not misleading anymore. -- To view, visit https://review.coreboot.org/c/flashrom/+/67310 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I097dd59f69c3fb532ac136796fcf5cae8839af7b Gerrit-Change-Number: 67310 Gerrit-PatchSet: 4 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Idwer Vollering <vidwer(a)gmail.com> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Comment-Date: Wed, 07 Sep 2022 00:06:18 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Thomas Heijligen <src(a)posteo.de> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: tests: Add workaround to allow tests mock fileno on FreeBSD
by Anastasia Klimchuk (Code Review) 06 Sep '22

06 Sep '22

Attention is currently required from: Edward O'Callaghan, Anastasia Klimchuk. Hello Felix Singer, build bot (Jenkins), Thomas Heijligen, Peter Marheine, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/67312 to look at the new patch set (#4). Change subject: tests: Add workaround to allow tests mock fileno on FreeBSD ...................................................................... tests: Add workaround to allow tests mock fileno on FreeBSD fileno can be a function and a macro in FreeBSD, depending on whether the environment in multi-threaded or single-threaded. For single thread, macro is used. Macro is expanded into a inline code which accesses private field of file descriptor. This is impossible to mock in tests. Pretending that environment is multi-threaded makes fileno function to be called instead of a macro. Function can be mocked for tests. BUG=b:237606255 TEST=ninja test tests pass on two environments: 1) FreeBSD 13.1-RELEASE-p2 GENERIC amd64 2) Debian 5.17.11 x86_64 GNU/Linux Change-Id: I3789ea9107a4cf8033cf59bb96d3c82aa58de026 TICKET: https://ticket.coreboot.org/issues/411 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M tests/chip.c 1 file changed, 37 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/12/67312/4 -- To view, visit https://review.coreboot.org/c/flashrom/+/67312 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3789ea9107a4cf8033cf59bb96d3c82aa58de026 Gerrit-Change-Number: 67312 Gerrit-PatchSet: 4 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-CC: Idwer Vollering <vidwer(a)gmail.com> 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]: tests: Guard Linux-specific linux/spidev.h header file
by Anastasia Klimchuk (Code Review) 06 Sep '22

06 Sep '22

Attention is currently required from: Anastasia Klimchuk. Hello Felix Singer, build bot (Jenkins), Thomas Heijligen, Angel Pons, Peter Marheine, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/67311 to look at the new patch set (#4). Change subject: tests: Guard Linux-specific linux/spidev.h header file ...................................................................... tests: Guard Linux-specific linux/spidev.h header file This allows unit tests to build on non-Linux platforms. BUG=b:237606255 TEST=unit tests compile on two environments: 1) FreeBSD 13.1-RELEASE-p2 GENERIC amd64 2) Debian 5.17.11 x86_64 GNU/Linux Change-Id: Icdb1b0cb29c5d62187153788ad5e0e631e8d0b62 TICKET: https://ticket.coreboot.org/issues/411 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M tests/lifecycle.h 1 file changed, 20 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/11/67311/4 -- To view, visit https://review.coreboot.org/c/flashrom/+/67311 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Icdb1b0cb29c5d62187153788ad5e0e631e8d0b62 Gerrit-Change-Number: 67311 Gerrit-PatchSet: 4 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Idwer Vollering <vidwer(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

[S] Change in flashrom[master]: tests: Use MOCK_FD instead of NON_ZERO for file operations
by Anastasia Klimchuk (Code Review) 06 Sep '22

06 Sep '22

Attention is currently required from: Anastasia Klimchuk. Hello Felix Singer, build bot (Jenkins), Thomas Heijligen, Angel Pons, Peter Marheine, Alexander Goncharov, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/67310 to look at the new patch set (#4). Change subject: tests: Use MOCK_FD instead of NON_ZERO for file operations ...................................................................... tests: Use MOCK_FD instead of NON_ZERO for file operations NON_ZERO can be a negative number, so MOCK_FD is safer option to use as a mock file descriptor. Also it is more readable. BUG=b:237606255 TEST=ninja test (on linux) Change-Id: I097dd59f69c3fb532ac136796fcf5cae8839af7b TICKET: https://ticket.coreboot.org/issues/411 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M tests/tests.c 1 file changed, 19 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/10/67310/4 -- To view, visit https://review.coreboot.org/c/flashrom/+/67310 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I097dd59f69c3fb532ac136796fcf5cae8839af7b Gerrit-Change-Number: 67310 Gerrit-PatchSet: 4 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Idwer Vollering <vidwer(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

[M] Change in flashrom[master]: Documentation: Add build instructions for meson
by Anastasia Klimchuk (Code Review) 06 Sep '22

06 Sep '22

Attention is currently required from: Felix Singer, Thomas Heijligen, Edward O'Callaghan. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/66862 ) Change subject: Documentation: Add build instructions for meson ...................................................................... Patch Set 5: Code-Review+1 (2 comments) File Documentation/building.md: https://review.coreboot.org/c/flashrom/+/66862/comment/59c715d2_eaaac030 PS5, Line 31: meson build -D<your_options> I was confused at first, because meson has `configure` command, which is not the same as build, and can be used to re-configure existing build directory. > meson configure -D<your_options> If you meant `build` here as a build directory and not a command, let's maybe call it `builddir` (here and below) ? https://review.coreboot.org/c/flashrom/+/66862/comment/efc6127b_777e50fd PS5, Line 47: tests Should this be `test` (not `tests`) ? I always run `ninja test` command. -- To view, visit https://review.coreboot.org/c/flashrom/+/66862 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3476f17fa274cd71e3e0e84f791d547d08165ecb Gerrit-Change-Number: 66862 Gerrit-PatchSet: 5 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> 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-CC: Felix Singer <felixsinger(a)posteo.net> 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-Comment-Date: Tue, 06 Sep 2022 22:53:50 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: meson: Overhaul the print_wiki option
by Anastasia Klimchuk (Code Review) 06 Sep '22

06 Sep '22

Attention is currently required from: Thomas Heijligen, Angel Pons. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/66703 ) Change subject: meson: Overhaul the print_wiki option ...................................................................... Patch Set 6: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/66703 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ic6c959b8b64ec2756b4535bd1b3320860f836aa5 Gerrit-Change-Number: 66703 Gerrit-PatchSet: 6 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> 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-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Tue, 06 Sep 2022 22:44:54 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: meson: Build the classic cli / flashrom cli executable as feature
by Anastasia Klimchuk (Code Review) 06 Sep '22

06 Sep '22

Attention is currently required from: Felix Singer, Thomas Heijligen, Edward O'Callaghan, Angel Pons. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/66705 ) Change subject: meson: Build the classic cli / flashrom cli executable as feature ...................................................................... Patch Set 6: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/66705 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: If39981ad0b3281edd6fc7e220362d2938dddf814 Gerrit-Change-Number: 66705 Gerrit-PatchSet: 6 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> 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-CC: Felix Singer <felixsinger(a)posteo.net> 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: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Tue, 06 Sep 2022 22:38:15 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit September 2022