Attention is currently required from: Edward O'Callaghan.
Daniel Campello has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62318 )
Change subject: tests/linux_mtd: Allow for checking open() calls
......................................................................
Patch Set 4:
(1 comment)
Patchset:
PS4:
Superseded by CB:62320
--
To view, visit https://review.coreboot.org/c/flashrom/+/62318
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I06e54c0bdc4f5320904e2ab6542345721f1ca370
Gerrit-Change-Number: 62318
Gerrit-PatchSet: 4
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Daniel Campello <campello(a)chromium.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Comment-Date: Thu, 31 Mar 2022 04:32:58 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Edward O'Callaghan.
Daniel Campello has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62311 )
Change subject: tests/realtek_mst: Allow test to check multiple paths
......................................................................
Patch Set 8:
(1 comment)
Patchset:
PS8:
Superseded by CB:63227
--
To view, visit https://review.coreboot.org/c/flashrom/+/62311
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I80d5e1fc26ad9caf49a98a7671d069bbd8428045
Gerrit-Change-Number: 62311
Gerrit-PatchSet: 8
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Daniel Campello <campello(a)chromium.org>
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: Thu, 31 Mar 2022 04:31:54 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Edward O'Callaghan, Anastasia Klimchuk.
Daniel Campello has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62312 )
Change subject: tests/: wrap flock() and ftruncate()
......................................................................
Patch Set 9:
(2 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/62312/comment/df553a68_60fcade3
PS6, Line 7: wrap flock() and ftruncate()
> To this point I think this should be squashed with CB:62213
Done
Patchset:
PS9:
Squashed into CB:62213
--
To view, visit https://review.coreboot.org/c/flashrom/+/62312
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I57e7843a4d94c0a7200690d4c1d9e4194b9f83c5
Gerrit-Change-Number: 62312
Gerrit-PatchSet: 9
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Daniel Campello <campello(a)chromium.org>
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-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Thu, 31 Mar 2022 04:30:39 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Daniel Campello <campello(a)chromium.org>
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Edward O'Callaghan, Anastasia Klimchuk.
Daniel Campello has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62324 )
Change subject: tests/chips.c: Mock out open() for lock file
......................................................................
Patch Set 6:
(5 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/62324/comment/f30a75d4_fa63767f
PS3, Line 7: out
> You can remove "out". Just "Mock open() for all chip tests". […]
Ack
Patchset:
PS6:
Squashed into CB:62320
File tests/chip.c:
https://review.coreboot.org/c/flashrom/+/62324/comment/6a1f7efd_9fc4626b
PS3, Line 212: data
> Similarly as in previous patch, this can be `erase_chip_io_state` and same for the rest of the patch […]
Ack
https://review.coreboot.org/c/flashrom/+/62324/comment/9fc819ad_41fa69da
PS3, Line 213: "/run/lock/flashrom_lock"
> If this is exactly identical path for all the chip tests, can be a macro in this file?
Done
https://review.coreboot.org/c/flashrom/+/62324/comment/eef71adf_706f71f8
PS3, Line 222: io_mock_register(&chip_io);
> You need to do `io_mock_register(NULL);` at the end of test, like verify tests do.
Done
--
To view, visit https://review.coreboot.org/c/flashrom/+/62324
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I527a266233648b0eef11a89108e82d0a008eeb8d
Gerrit-Change-Number: 62324
Gerrit-PatchSet: 6
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Daniel Campello <campello(a)chromium.org>
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-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Thu, 31 Mar 2022 04:29:55 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Edward O'Callaghan.
Hello build bot (Jenkins), Edward O'Callaghan,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/63227
to look at the new patch set (#6).
Change subject: tests: assert pathname and flags when calling open()
......................................................................
tests: assert pathname and flags when calling open()
With this change the wrappers for mock and friends are able to take an
optional io_mock_open_state struct to assert expected pathnames and
flags whenever an open operation is called.
Based partially on https://review.coreboot.org/c/flashrom/+/62319/5
BUG=b:227404721,b:217629892,b:215255210
TEST=./test_build.sh; FEATURES=test emerge-amd64-generic flashrom
BRANCH=none
Signed-off-by: Daniel Campello <campello(a)chromium.org>
Co-Author: Edward O'Callaghan <quasisec(a)google.com>
Change-Id: Ib46ca5b854c8453ec02ae09f3151cd4d25f988eb
---
M tests/io_mock.h
M tests/lifecycle.c
M tests/tests.c
3 files changed, 37 insertions(+), 17 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/27/63227/6
--
To view, visit https://review.coreboot.org/c/flashrom/+/63227
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ib46ca5b854c8453ec02ae09f3151cd4d25f988eb
Gerrit-Change-Number: 63227
Gerrit-PatchSet: 6
Gerrit-Owner: Daniel Campello <campello(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-MessageType: newpatchset
Attention is currently required from: Edward O'Callaghan, Anastasia Klimchuk.
Daniel Campello has uploaded a new patch set (#9) to the change originally created by Edward O'Callaghan. ( https://review.coreboot.org/c/flashrom/+/62319 )
Change subject: tests/linux_spi: Validate param file path
......................................................................
tests/linux_spi: Validate param file path
Add path validation for '/dev/null' to open operation.
BUG=b:217629892,b:215255210
TEST=`ninja test`.
Signed-off-by: Edward O'Callaghan <quasisec(a)google.com>
Co-Author: Daniel Campello <campello(a)chromium.org>
Signed-off-by: Daniel Campello <campello(a)chromium.org>
Change-Id: If5d24c65f291c53a35509fea5d2f5b3fdb51c306
---
M tests/lifecycle.c
1 file changed, 6 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/19/62319/9
--
To view, visit https://review.coreboot.org/c/flashrom/+/62319
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: If5d24c65f291c53a35509fea5d2f5b3fdb51c306
Gerrit-Change-Number: 62319
Gerrit-PatchSet: 9
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Daniel Campello <campello(a)chromium.org>
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-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: newpatchset
Attention is currently required from: Edward O'Callaghan, Anastasia Klimchuk.
Daniel Campello has uploaded a new patch set (#9) to the change originally created by Edward O'Callaghan. ( https://review.coreboot.org/c/flashrom/+/62320 )
Change subject: tests/: Add file path validation to open() calls
......................................................................
tests/: Add file path validation to open() calls
With this change we add path validation to many tests that do not call
open. Expected path is set to NULL, if the code indead calls open then
the assertion for non-NULL will make the test fail.
BUG=b:217629892,b:215255210
TEST=`ninja test`.
Change-Id: I892fa1ecee26ebce9640893edbb228fa9aa7b0b6
Signed-off-by: Edward O'Callaghan <quasisec(a)google.com>
Co-Author: Daniel Campello <campello(a)chromium.org>
Signed-off-by: Daniel Campello <campello(a)chromium.org>
---
M tests/chip.c
M tests/lifecycle.c
2 files changed, 121 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/20/62320/9
--
To view, visit https://review.coreboot.org/c/flashrom/+/62320
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I892fa1ecee26ebce9640893edbb228fa9aa7b0b6
Gerrit-Change-Number: 62320
Gerrit-PatchSet: 9
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Daniel Campello <campello(a)chromium.org>
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-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: newpatchset
Attention is currently required from: Hung-Te Lin, Nico Huber, Subrata Banik, Edward O'Callaghan.
Daniel Campello has uploaded a new patch set (#15) to the change originally created by Edward O'Callaghan. ( https://review.coreboot.org/c/flashrom/+/62213 )
Change subject: flashrom.c: Implement file-based locking semantics
......................................................................
flashrom.c: Implement file-based locking semantics
This upstreams the ChromiumOS implementation of file-based locking
for multiple instances of flashrom that could be spawned either
from libflashrom (perhaps by fwupd for example) and user cli
as another example. Since flashrom is programming singleton state
of hardware from userspace there is no way to exclusively own
the address space and therefore a file-based locking semantic
is considered here.
BUG=b:217629892,b:215255210
BRANCH=none
TEST=nm -gD /build/brya/usr/lib64/libflashrom.so | grep "flock"
Test futility update with multiple instances of flashrom running.
Change-Id: I19cb4e3bf14caeb67c3e8100a20395b264c5113a
Signed-off-by: Edward O'Callaghan <quasisec(a)google.com>
---
M Makefile
A big_lock.c
A big_lock.h
A file_lock.c
M flashrom.c
A ipc_lock.h
M meson.build
M tests/chip.c
M tests/include/test.h
M tests/lifecycle.c
M tests/meson.build
M tests/tests.c
12 files changed, 498 insertions(+), 17 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/13/62213/15
--
To view, visit https://review.coreboot.org/c/flashrom/+/62213
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I19cb4e3bf14caeb67c3e8100a20395b264c5113a
Gerrit-Change-Number: 62213
Gerrit-PatchSet: 15
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-MessageType: newpatchset
Attention is currently required from: Edward O'Callaghan, Anastasia Klimchuk.
Daniel Campello has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62320 )
Change subject: tests/: Allow for file path validation
......................................................................
Patch Set 8:
(5 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/62320/comment/f69d298b_8888475b
PS4, Line 7: Allow for file path validation
> I added some comments about commit messages in previous patches, this one would need to be aligned a […]
Again, no multiple paths even when they are supported by the framework.
File tests/init_shutdown.c:
https://review.coreboot.org/c/flashrom/+/62320/comment/12992ca2_a0c86ca4
PS4, Line 61: data
> Why dummy needs mocks? This is unexpected [to me].
This is actually making sure that no open() calls are made.
https://review.coreboot.org/c/flashrom/+/62320/comment/cb76970d_d3c21b63
PS4, Line 80: data
> Same comment as in previous patches: let's be more specific with naming and call this `nicrealtek_io […]
Done
https://review.coreboot.org/c/flashrom/+/62320/comment/e19c58ea_5ef5005f
PS4, Line 89:
> Lets add new line before and after `io_mock_register`
Done
https://review.coreboot.org/c/flashrom/+/62320/comment/b35a2d1b_9d12bac8
PS4, Line 90:
> Since this test now registering io mock, it also needs to unregister, which is […]
Done
--
To view, visit https://review.coreboot.org/c/flashrom/+/62320
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I892fa1ecee26ebce9640893edbb228fa9aa7b0b6
Gerrit-Change-Number: 62320
Gerrit-PatchSet: 8
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Daniel Campello <campello(a)chromium.org>
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-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Thu, 31 Mar 2022 04:04:16 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Edward O'Callaghan, Anastasia Klimchuk.
Daniel Campello has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62319 )
Change subject: tests/linux_spi: Validate param file path
......................................................................
Patch Set 8:
(7 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/62319/comment/fe852565_a54d1f83
PS4, Line 7: Validate param file path
> Sorry I missed in the first batch of comments. […]
Even when the platform supports multiple paths (for multiple calls to open) here there is a single call with a single path. I think title is descriptive as is.
https://review.coreboot.org/c/flashrom/+/62319/comment/a8bdd83e_99e3dbfd
PS4, Line 10: linux_spi unit-test does not
: check
> I think unit-tests in general had no implementation to mock repeated calls to open, it is not specif […]
Done
File tests/init_shutdown.c:
https://review.coreboot.org/c/flashrom/+/62319/comment/f674532d_e6cc0d60
PS4, Line 39: #define DEFAULT_MOCK_FD 0xC0FE
> You can rebase on top of https://review.coreboot. […]
Done
https://review.coreboot.org/c/flashrom/+/62319/comment/500d2b87_cdf3d152
PS4, Line 41: default_io_mock_state
> I have rebased with 'fallback' instead. […]
Ack
https://review.coreboot.org/c/flashrom/+/62319/comment/89a096a7_dbfbaa58
PS4, Line 48: default_open
> For the same reasons as above, I would rename this to something more self-explanatory, maybe "multip […]
Ack
https://review.coreboot.org/c/flashrom/+/62319/comment/52049d09_85732e28
PS4, Line 305: data
> Ok, this is existing pattern that was reviewed and approved by everyone, that's why I suggested it. […]
Done
File tests/wraps.c:
https://review.coreboot.org/c/flashrom/+/62319/comment/b58c15bc_ec39dd43
PS5, Line 26: assert_int_equal(flags & io_state->flags, io_state->flags);
> flags may be different for each path
Done
--
To view, visit https://review.coreboot.org/c/flashrom/+/62319
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: If5d24c65f291c53a35509fea5d2f5b3fdb51c306
Gerrit-Change-Number: 62319
Gerrit-PatchSet: 8
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Daniel Campello <campello(a)chromium.org>
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-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Thu, 31 Mar 2022 04:00:04 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org>
Comment-In-Reply-To: Daniel Campello <campello(a)chromium.org>
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: comment