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

Change in flashrom[master]: meson: fix dependency on raw access
by Nico Huber (Code Review) 29 Mar '21

29 Mar '21

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/51577 ) Change subject: meson: fix dependency on raw access ...................................................................... Patch Set 3: (1 comment) File meson.build: https://review.coreboot.org/c/flashrom/+/51577/comment/8078d892_7732d1cb PS3, Line 280: need_raw_access = true > this change makes meson to behave the same as the Makefile. No, it doesn't. The Makefile states that rayer_spi needs raw access but not that it needs libpci. Hence I was asking. -- To view, visit https://review.coreboot.org/c/flashrom/+/51577 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id3fa4ec7a735b81e989ba9fe2b53b18d0956627a Gerrit-Change-Number: 51577 Gerrit-PatchSet: 3 Gerrit-Owner: Daniel Campello <campello(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Mario Limonciello <superm1(a)gmail.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Richard Hughes <hughsient(a)gmail.com> Gerrit-Reviewer: Richard Hughes <richard(a)hughsie.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Comment-Date: Mon, 29 Mar 2021 19:05:41 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Daniel Campello <campello(a)chromium.org> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: meson: fix dependency on raw access
by Daniel Campello (Code Review) 29 Mar '21

29 Mar '21

Daniel Campello has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/51577 ) Change subject: meson: fix dependency on raw access ...................................................................... Patch Set 3: (1 comment) File meson.build: https://review.coreboot.org/c/flashrom/+/51577/comment/996aec40_d7b1403c PS3, Line 280: need_raw_access = true > Smells like improper dependency handling in meson. Indeed. Makefile was fine, meson was not, this change makes meson to behave the same as the Makefile. -- To view, visit https://review.coreboot.org/c/flashrom/+/51577 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id3fa4ec7a735b81e989ba9fe2b53b18d0956627a Gerrit-Change-Number: 51577 Gerrit-PatchSet: 3 Gerrit-Owner: Daniel Campello <campello(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Mario Limonciello <superm1(a)gmail.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Richard Hughes <hughsient(a)gmail.com> Gerrit-Reviewer: Richard Hughes <richard(a)hughsie.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Comment-Date: Mon, 29 Mar 2021 16:39:08 +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]: tree: Remove forward-declarations of structs for spi masters
by Edward O'Callaghan (Code Review) 29 Mar '21

29 Mar '21

Attention is currently required from: Nico Huber, Anastasia Klimchuk. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/51731 ) Change subject: tree: Remove forward-declarations of structs for spi masters ...................................................................... Patch Set 1: Code-Review+2 (1 comment) Patchset: PS1: > A good test for such changes is if it results in the […] +1 to Nico's suggestion, that is precisely how we check these things. If already done, just set the TEST= line to indicate a before,after objdump compare was performed to verify no change. -- To view, visit https://review.coreboot.org/c/flashrom/+/51731 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I256bd7c763efc010fc1f29f7c5853f150ac10739 Gerrit-Change-Number: 51731 Gerrit-PatchSet: 1 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-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Mon, 29 Mar 2021 13:18:51 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: Add unit test to run init/shutdown for enabled drivers
by Anastasia Klimchuk (Code Review) 29 Mar '21

29 Mar '21

Attention is currently required from: Nico Huber, Angel Pons. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/51487 ) Change subject: Add unit test to run init/shutdown for enabled drivers ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: > As you can see from the amount of tests, it's a rather new topic for […] Thank you! I will try to address your other comment to create mocks in separate compilation unit. I am working on understanding how to do it. Good thing I need to do this hard thing once, and then just write more tests. Re: flashbuses_to_text_test_success() I agree. I am not touching it yet because it is an existing test. But ideally I would find what place is using those strings and test that place. In other words, what is going to fail if the string is different? -- To view, visit https://review.coreboot.org/c/flashrom/+/51487 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3af612defe1af3850dfc1626a208d873e3a3eddc Gerrit-Change-Number: 51487 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Mon, 29 Mar 2021 05:17:33 +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> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: Accept shutdown function in register_master
by Anastasia Klimchuk (Code Review) 29 Mar '21

29 Mar '21

Attention is currently required from: Angel Pons. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/51761 ) Change subject: Accept shutdown function in register_master ...................................................................... Patch Set 3: (1 comment) Patchset: PS3: Thanks heaps for +2s this is so cool! I am currently working on the next patch in the series, which implements register_shutdown in register_master and takes care of memory management for data (data being second parameter in register_shutdown). I wanted to get the next patch in working state before merging this, just in case any changes would be needed here (size of data for example). So I would hold this one from merging until I get the next one, hope it’s fine. -- To view, visit https://review.coreboot.org/c/flashrom/+/51761 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I1704ddafc56e1393613a7ea077e40c36911cfac8 Gerrit-Change-Number: 51761 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: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-CC: Sam McNally <sammc(a)google.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Mon, 29 Mar 2021 05:04:21 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: Enable dynamic memory allocation checks for cmocka unit tests
by Anastasia Klimchuk (Code Review) 29 Mar '21

29 Mar '21

Attention is currently required from: Nico Huber, David Hendricks, Edward O'Callaghan. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/51243 ) Change subject: Enable dynamic memory allocation checks for cmocka unit tests ...................................................................... Patch Set 3: (3 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/51243/comment/19d7c988_e5f6139f PS2, Line 10: files and tests. > Nit. If these are paragraphs, split them with empty lines. If they […] done Patchset: PS2: > > > 1) There are two different headers for a file under test and for a test (unittest_env. […] Nico, sorry for a delay, I just needed some time to think about it. I really like the idea and want to do this, because it “leaves the code untouched” as you said. That would cover all the files implicitly, which is great: no need to add unittest_env.h in every file and what is even better all new future files are covered as well. Now I am trying to get a clear idea how exactly to implement this, so reading meson manual etc, definitely working on this. Let’s check my understanding: 1) I need to create another target in root (not test) meson.build, so that will be two targets: one for “real env”, one for “test env”? And then tests/meson.build will use that second “test env” target? 2) And “real env” target leaves includes as is, while “test env” target adds stdlib.h and unittest_env.h? 3) The last bit: tests/meson.build includes unittest.h in every test? I am resolving two other comments, just leaving remaining hard one, but I am working on it! File tests/flashrom.c: https://review.coreboot.org/c/flashrom/+/51243/comment/fcfe57a8_122bd274 PS2, Line 27: #define assert_string_not_equal_with_free(text, expected) \ > NB. I known this is probably just following cmocka, but I find it […] I tried to make it better, dropped _string (7 chars) and replaced free->and (1 char) so in total I removed 8 chars from boilerplate. The remaining stuff seems to be critical: assert and free, both meaningful words, I would keep them. What do you think? -- To view, visit https://review.coreboot.org/c/flashrom/+/51243 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0c6b6b8dc17aaee28640e3fca3d1fc9f7feabf5f Gerrit-Change-Number: 51243 Gerrit-PatchSet: 3 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Daniel Kurtz <djkurtz(a)google.com> Gerrit-Reviewer: David Hendricks <david.hendricks(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: Daniel Kurtz <djkurtz(a)chromium.org> Gerrit-CC: Thomas Heijligen <src(a)posteo.de> Gerrit-CC: Victor Ding <victording(a)chromium.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Mon, 29 Mar 2021 04:50:13 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: Enable dynamic memory allocation checks for cmocka unit tests
by Anastasia Klimchuk (Code Review) 29 Mar '21

29 Mar '21

Attention is currently required from: David Hendricks, Edward O'Callaghan, Anastasia Klimchuk. Hello build bot (Jenkins), Nico Huber, Daniel Kurtz, David Hendricks, Edward O'Callaghan, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/51243 to look at the new patch set (#3). Change subject: Enable dynamic memory allocation checks for cmocka unit tests ...................................................................... Enable dynamic memory allocation checks for cmocka unit tests This commit enables the feature and makes changes to existing files and tests. I am writing more new tests with this. Commit includes tests/flashrom.c because after enabling memory checks the test started to fail (it used to leak memory indeed). If you are wondering how to verify it works (because at the moment all tests [still] pass so it’s not obvious that anything has changed), then for example: 1) Remove free’s in flashbuses_to_text_test_success test, and it will fail with message similar to this (line numbers from your local source) [ ERROR ] --- Blocks allocated... ../flashrom.c:1239: note: block 0x55f42304b640 allocated here ../flashrom.c:1239: note: block 0x55f42304b5c0 allocated here ../flashrom.c:1239: note: block 0x55f42304b3d0 allocated here ../flashrom.c:1239: note: block 0x55f42304b700 allocated here ../flashrom.c:1239: note: block 0x55f42304b780 allocated here ../flashrom.c:1239: note: block 0x55f42304bb00 allocated here ../flashrom.c:1239: note: block 0x55f42304b810 allocated here ERROR: flashbuses_to_text_test_success leaked 7 block(s) 2) Add char *temp = malloc just before return from strcat_realloc [ ERROR ] --- Blocks allocated... ../helpers.c:88: note: block 0x55a51307b6c0 allocated here ../helpers.c:88: note: block 0x55a51307b9e0 allocated here ERROR: strcat_realloc_test_success leaked 2 block(s) BUG=b:181803212 TEST=builds and ninja test Change-Id: I0c6b6b8dc17aaee28640e3fca3d1fc9f7feabf5f Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M flashrom.c M helpers.c M tests/flashrom.c M tests/helpers.c M tests/meson.build A tests/unittest.h A unittest_env.h 7 files changed, 117 insertions(+), 12 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/43/51243/3 -- To view, visit https://review.coreboot.org/c/flashrom/+/51243 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0c6b6b8dc17aaee28640e3fca3d1fc9f7feabf5f Gerrit-Change-Number: 51243 Gerrit-PatchSet: 3 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Daniel Kurtz <djkurtz(a)google.com> Gerrit-Reviewer: David Hendricks <david.hendricks(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: Daniel Kurtz <djkurtz(a)chromium.org> Gerrit-CC: Thomas Heijligen <src(a)posteo.de> Gerrit-CC: Victor Ding <victording(a)chromium.org> Gerrit-Attention: David Hendricks <david.hendricks(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

Change in flashrom[master]: flashrom.c: automatic generated programmer_enum.h
by Xiang Wang (Code Review) 28 Mar '21

28 Mar '21

Attention is currently required from: Sam McNally, Nico Huber, Stefan Reinauer. Xiang Wang has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/49599 ) Change subject: flashrom.c: automatic generated programmer_enum.h ...................................................................... Patch Set 6: (1 comment) File Makefile: https://review.coreboot.org/c/flashrom/+/49599/comment/9a980303_92ebea9a PS4, Line 681: $(shell bash ./util/generator_programmer_enum.sh) > It's as I feared: If you run `make` two times in a row without this […] Please guide me how to modify the Makefile, and meson.build also needs to make the same modification. -- To view, visit https://review.coreboot.org/c/flashrom/+/49599 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3f4370ccae2b64da3c4178243b192700d3d205d2 Gerrit-Change-Number: 49599 Gerrit-PatchSet: 6 Gerrit-Owner: Xiang Wang <merle(a)hardenedlinux.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Sam McNally <sammc(a)google.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-CC: Idwer Vollering <vidwer(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-CC: Shawn C <citypw(a)hardenedlinux.org> Gerrit-Attention: Sam McNally <sammc(a)google.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Comment-Date: Sun, 28 Mar 2021 01:05:01 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Xiang Wang <merle(a)hardenedlinux.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: Accept shutdown function in register_master
by Nico Huber (Code Review) 25 Mar '21

25 Mar '21

Attention is currently required from: Angel Pons, Anastasia Klimchuk. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/51761 ) Change subject: Accept shutdown function in register_master ...................................................................... Patch Set 3: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/51761 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I1704ddafc56e1393613a7ea077e40c36911cfac8 Gerrit-Change-Number: 51761 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: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-CC: Sam McNally <sammc(a)google.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Thu, 25 Mar 2021 22:35:38 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: flashchips: Add AT25DF011
by Paul Menzel (Code Review) 25 Mar '21

25 Mar '21

Attention is currently required from: Angel Pons, Hanno Heinrichs. Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/51048 ) Change subject: flashchips: Add AT25DF011 ...................................................................... Patch Set 4: Code-Review+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/51048 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I5a2141f1380e864c843d6a3008fdb02dc1b75131 Gerrit-Change-Number: 51048 Gerrit-PatchSet: 4 Gerrit-Owner: Hanno Heinrichs <hanno.heinrichs(a)rwth-aachen.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Hanno Heinrichs <hanno.heinrichs(a)rwth-aachen.de> Gerrit-Comment-Date: Thu, 25 Mar 2021 18:17:50 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit March 2021