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

Change in flashrom[master]: programmer.h: Minor tidy up
by Nico Huber (Code Review) 30 Jul '21

30 Jul '21

Attention is currently required from: Edward O'Callaghan. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/56466 ) Change subject: programmer.h: Minor tidy up ...................................................................... Patch Set 2: (1 comment) Patchset: PS2: > Very likely not in most cases. […] Hmmm, looking at this from another angle: I think we should just split the file. That would make guarding implicit, right? e.g. if you some code doesn't need USB stuff, it wouldn't include the usb header file. If all these declarations can be clearly grouped, then I see no reason to have them in a monolithic header file. WDYT? -- To view, visit https://review.coreboot.org/c/flashrom/+/56466 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7ceda32b66d17318d42b94a1fab621a9a926fa77 Gerrit-Change-Number: 56466 Gerrit-PatchSet: 2 Gerrit-Owner: 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: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Fri, 30 Jul 2021 14:29:01 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: tests: Add tests to erase a chip
by Anastasia Klimchuk (Code Review) 30 Jul '21

30 Jul '21

Attention is currently required from: Nico Huber, Angel Pons. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/56501 ) Change subject: tests: Add tests to erase a chip ...................................................................... Patch Set 6: (1 comment) Patchset: PS2: > I looked again, and realised I want to improve my solution a bit. […] Some more explanations (sorry for spam :D) My strong preference to have only one flashrom_test_dep in root meson.build , because if there are two dependencies this creates a "choice" and a question "which one should I choose for my new test? which one is the right one?" and a chance to pick wrong one. There should not be a choice in this situation, only one flashrom_test_dep with all compile args set up for test context. A side-effect is that all test_outx and test_inx need to be wrapped for all tests executables, even if the test itself is not using them. Those wraps can be empty, but need to be present. Potentially later this can be improved, maybe wraps extracted into separate .c file. There are lots of wraps already, and there will be more (because there will be more tests!) -- To view, visit https://review.coreboot.org/c/flashrom/+/56501 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I6f74bfe4e02244d24d6c837cc3d551251e7b4898 Gerrit-Change-Number: 56501 Gerrit-PatchSet: 6 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: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Fri, 30 Jul 2021 06:12:42 +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]: tests: Add tests to erase a chip
by Anastasia Klimchuk (Code Review) 30 Jul '21

30 Jul '21

Attention is currently required from: Nico Huber, Angel Pons. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/56501 ) Change subject: tests: Add tests to erase a chip ...................................................................... Patch Set 6: (1 comment) Patchset: PS2: > Yes, I remember that when function is wrapped, the real one is meant to be available as __real_funct […] I looked again, and realised I want to improve my solution a bit. I don't want any changes in root meson.build (reverted them), so now all changes are contained within tests/ Still two test executables, but both of them using the same flashrom_test_dep from root meson.build. The thing which is not ideal: wraps for inx and outx are repeated in wrap_send_tests.c (they can do nothing, but need to be present). I am thinking how to make this better... but maybe this can be done later? (if everyone agrees of course). Next thing I am going to split the patch into two. -- To view, visit https://review.coreboot.org/c/flashrom/+/56501 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I6f74bfe4e02244d24d6c837cc3d551251e7b4898 Gerrit-Change-Number: 56501 Gerrit-PatchSet: 6 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: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Fri, 30 Jul 2021 05:40:10 +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]: tests: Add tests to erase a chip
by Anastasia Klimchuk (Code Review) 30 Jul '21

30 Jul '21

Attention is currently required from: Nico Huber, 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/+/56501 to look at the new patch set (#6). Change subject: tests: Add tests to erase a chip ...................................................................... tests: Add tests to erase a chip Two tests cover the code which performs do_erase operation. First one works with fake chip and dummy programmer. Fake chip has all operations defined, and a buffer to emulate chip memory. Second one uses the chip which is closer to the real one, because read/write/unlock/erase operations are real. The tests takes the advantage of dummyflasher's capability of emulating a W25Q128.V chip. BUG=b:181803212 TEST=builds and ninja test Change-Id: I6f74bfe4e02244d24d6c837cc3d551251e7b4898 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- A tests/chip.c M tests/meson.build M tests/tests.c M tests/tests.h A tests/wrap_send_tests.c A tests/wrap_send_tests.h 6 files changed, 365 insertions(+), 34 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/01/56501/6 -- To view, visit https://review.coreboot.org/c/flashrom/+/56501 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I6f74bfe4e02244d24d6c837cc3d551251e7b4898 Gerrit-Change-Number: 56501 Gerrit-PatchSet: 6 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: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> 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]: flashchips: Add MX25L12833F
by Nico Huber (Code Review) 29 Jul '21

29 Jul '21

Attention is currently required from: Henry Sun, Tao Xia, Angel Pons, Anastasia Klimchuk, Nikolai Artemiev, Karthik Ramasubramanian. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/56546 ) Change subject: flashchips: Add MX25L12833F ...................................................................... Patch Set 1: (2 comments) Patchset: PS1: > I have uploaded the datasheet in the issue https://partnerissuetracker.corp.google. […] This bug tracker is not public, though. Found it here: https://www.mxic.com.tw/Lists/Datasheet/Attachments/7447/MX25L12833F,%203V,… File flashchips.c: https://review.coreboot.org/c/flashrom/+/56546/comment/f86b40e7_3c0923b4 PS1, Line 8368: 512B Looks like the 33F has a whole kilobyte? -- To view, visit https://review.coreboot.org/c/flashrom/+/56546 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I14ab7e04f5209d2bcf34b0d2de9da2c01bf32d00 Gerrit-Change-Number: 56546 Gerrit-PatchSet: 1 Gerrit-Owner: Tao Xia <xiatao5(a)huaqin.corp-partner.google.com> 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: Henry Sun <henrysun(a)google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Ginger Zhang <zhangqingchun(a)huaqin.corp-partner.google.com> Gerrit-CC: Krystal Han <hanlijing(a)huaqin.corp-partner.google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Weimin Wu <wuweimin(a)huaqin.corp-partner.google.com> Gerrit-Attention: Henry Sun <henrysun(a)google.com> Gerrit-Attention: Tao Xia <xiatao5(a)huaqin.corp-partner.google.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Comment-Date: Thu, 29 Jul 2021 09:58:42 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Tao Xia <xiatao5(a)huaqin.corp-partner.google.com> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ft2232_spi: Add FTDI search by description.
by Nico Huber (Code Review) 29 Jul '21

29 Jul '21

Nico Huber has submitted this change. ( https://review.coreboot.org/c/flashrom/+/56164 ) Change subject: ft2232_spi: Add FTDI search by description. ...................................................................... ft2232_spi: Add FTDI search by description. This adds to the search-by-serial functionality with search-by-description (product string). This is useful when e.g. one has multiple FTDIs in a system and wants the serial numbers to reflect the system-level serial number, and the description to reflect the subcomponent names. Tested manually by running with both serial and description searches, on a machine with multiple FTDIs plugged in. Ensured that when two devices with the same vid/pid/serial number are plugged in, description can be used to differentiate. Verifed no-description, no-serial, one FTDI plugged in base case works. Original version of this code used the original single "arg" char*, but on further thought, this wasn't worth the readability and functionality losses. The new version with arg2 gets rid of several lines of code, the gotos, and adds the ability to filter by both description and serial simultaneously. Change-Id: Ib4be23247995710900175f5f16e38db577ef08fa Signed-off-by: Harry Johnson <johnsonh(a)waymo.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/56164 Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M ft2232_spi.c 1 file changed, 6 insertions(+), 2 deletions(-) Approvals: build bot (Jenkins): Verified Anastasia Klimchuk: Looks good to me, approved diff --git a/ft2232_spi.c b/ft2232_spi.c index d853b09..e32d7f8 100644 --- a/ft2232_spi.c +++ b/ft2232_spi.c @@ -319,7 +319,7 @@ */ uint32_t divisor = DEFAULT_DIVISOR; int f; - char *arg; + char *arg, *arg2; double mpsse_clk; uint8_t cs_bits = 0x08; @@ -499,8 +499,12 @@ } arg = extract_programmer_param("serial"); - f = ftdi_usb_open_desc(&ftdic, ft2232_vid, ft2232_type, NULL, arg); + arg2 = extract_programmer_param("description"); + + f = ftdi_usb_open_desc(&ftdic, ft2232_vid, ft2232_type, arg2, arg); + free(arg); + free(arg2); if (f < 0 && f != -5) { msg_perr("Unable to open FTDI device: %d (%s)\n", f, -- To view, visit https://review.coreboot.org/c/flashrom/+/56164 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ib4be23247995710900175f5f16e38db577ef08fa Gerrit-Change-Number: 56164 Gerrit-PatchSet: 6 Gerrit-Owner: johnsonh(a)waymo.com Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Andrew Bresticker <abrestic(a)google.com> Gerrit-MessageType: merged

1 0

Change in flashrom[master]: tests: Add tests to erase a chip
by Edward O'Callaghan (Code Review) 29 Jul '21

29 Jul '21

Attention is currently required from: Nico Huber, Angel Pons, Anastasia Klimchuk. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/56501 ) Change subject: tests: Add tests to erase a chip ...................................................................... Patch Set 5: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/56501 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I6f74bfe4e02244d24d6c837cc3d551251e7b4898 Gerrit-Change-Number: 56501 Gerrit-PatchSet: 5 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: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Thu, 29 Jul 2021 04:46:11 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: spi_master: Use new API to register shutdown function
by Miklós Márton (Code Review) 28 Jul '21

28 Jul '21

Attention is currently required from: Nico Huber, Thomas Heijligen, Angel Pons, Anastasia Klimchuk. Miklós Márton has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/56103 ) Change subject: spi_master: Use new API to register shutdown function ...................................................................... Patch Set 4: (1 comment) Patchset: PS1: > Miklós thanks heaps!! I assume since you +2 that means testing successful? (so I resolve this commen […] Yes I managed to build and test the ni845x_spi with this patch (with compiling with WARNERROR=no due to the signed unsigned comaprison warnings in the ni845x_spi code but thats an another story). I have no objections on the merge from the ni845x_spi side. -- To view, visit https://review.coreboot.org/c/flashrom/+/56103 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ib60300f9ddb295a255d5ef3f8da0e07064207140 Gerrit-Change-Number: 56103 Gerrit-PatchSet: 4 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: Miklós Márton <martonmiklosqdev(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.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: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Wed, 28 Jul 2021 08:43:20 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Miklós Márton <martonmiklosqdev(a)gmail.com> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: spi_master: Use new API to register shutdown function
by Anastasia Klimchuk (Code Review) 28 Jul '21

28 Jul '21

Attention is currently required from: Nico Huber, Thomas Heijligen, Angel Pons. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/56103 ) Change subject: spi_master: Use new API to register shutdown function ...................................................................... Patch Set 4: (2 comments) Patchset: PS1: > Hello Anastasia, […] Miklós thanks heaps!! I assume since you +2 that means testing successful? (so I resolve this comment) Patchset: PS4: I rebased this one and previous patches in the chain, so it is nice and ready :) This would not happen without all your help! -- To view, visit https://review.coreboot.org/c/flashrom/+/56103 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ib60300f9ddb295a255d5ef3f8da0e07064207140 Gerrit-Change-Number: 56103 Gerrit-PatchSet: 4 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: Miklós Márton <martonmiklosqdev(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.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: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Wed, 28 Jul 2021 03:50:40 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Miklós Márton <martonmiklosqdev(a)gmail.com> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: tests: Add tests to erase a chip
by Anastasia Klimchuk (Code Review) 28 Jul '21

28 Jul '21

Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/56501 ) Change subject: tests: Add tests to erase a chip ...................................................................... Patch Set 5: (6 comments) Patchset: PS2: > > The newest test that I have added uses real spi_send_command, however some previous tests are mock […] Yes, I remember that when function is wrapped, the real one is meant to be available as __real_function. Which would work if the function is explicitly called in test code, but in this case spi_send_command is called somewhere deep inside in the middle of do_erase operation, the test doesn't know when and how (and shouldn't know). I did spend some time few months ago exploring wrap vs real situation, when I was writing my first tests. I never sent those very first tests though, because they were wrapping register_shutdown, and that was a conflict with init-shutdown tests that do real shutdown. When I was trying to alternate between wrap and mock in different tests (in the same executable), the code was quickly getting convoluted, but I couldn't make it work anyway :\ (searching the internet didn't help) I decided init-shutdown tests were more useful and chose them. That was few months ago, when I was younger and less experienced (especially the latter), so maybe now I could make it work... but the code would have to be convoluted, I am worried about it a bit. I am marking this comment unresolved, because there is a TODO for this patch one way or the other. Myself leaning to have two executables, but please tell me if you strongly disagree! Patchset: PS4: > Definitely split this like you suggested but the first test looks great now! thanks! File tests/chip.c: https://review.coreboot.org/c/flashrom/+/56501/comment/d5062766_b7fd379b PS4, Line 25: int > `unsigned` unless negative unlocks encode something special? Done https://review.coreboot.org/c/flashrom/+/56501/comment/fe0f1521_98143b4f PS4, Line 52: memcpy(&g_chip_state.buf[start], buf, len); > validate len does not exceed the actual buffer length. […] Done https://review.coreboot.org/c/flashrom/+/56501/comment/cb0eca80_5dd8a180 PS4, Line 59: g_chip_state.unlock_calls++; : return 0; > this does depend on the chip I suppose but another possible behavior is the following: […] I added this check, it makes sense to me. For the existing test, unlock only called once (we know because it is logged). If at some point in future this becomes an issue, we can have a look later then. https://review.coreboot.org/c/flashrom/+/56501/comment/499a6fc9_f7449f95 PS4, Line 71: memset(&g_chip_state.buf[blockaddr], 0xff, blocklen); > ditto, ensure blocklen does not exceed the global buffer size. Done -- To view, visit https://review.coreboot.org/c/flashrom/+/56501 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I6f74bfe4e02244d24d6c837cc3d551251e7b4898 Gerrit-Change-Number: 56501 Gerrit-PatchSet: 5 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: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Wed, 28 Jul 2021 03:20:55 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> 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 July 2021