October 2021 - flashrom-gerrit

Change in flashrom[master]: tests: Fix fread wrap to return success by default
by Nico Huber (Code Review) 19 Oct '21

19 Oct '21

Nico Huber has submitted this change. ( https://review.coreboot.org/c/flashrom/+/58356 ) Change subject: tests: Fix fread wrap to return success by default ...................................................................... tests: Fix fread wrap to return success by default Successful return value for fread is the number of items read, and default behaviour for all wraps is to return success. This worked previously because all existing tests have custom mocks for fread, so default behaviour hasn't been used. However next patch in this chain adds new test which needs default fread wrap. BUG=b:181803212 TEST=ninja test Change-Id: I17d82d281a87129843f547b87c18f52aca23314d Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/58356 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Nico Huber <nico.h(a)gmx.de> --- M tests/tests.c 1 file changed, 3 insertions(+), 3 deletions(-) Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved diff --git a/tests/tests.c b/tests/tests.c index 3eb3129..d3df356 100644 --- a/tests/tests.c +++ b/tests/tests.c @@ -168,12 +168,12 @@ return NULL; } -size_t __wrap_fread(void *ptr, size_t size, size_t len, FILE *fp) +size_t __wrap_fread(void *ptr, size_t size, size_t nmemb, FILE *fp) { LOG_ME; if (get_io() && get_io()->fread) - return get_io()->fread(get_io()->state, ptr, size, len, fp); - return 0; + return get_io()->fread(get_io()->state, ptr, size, nmemb, fp); + return nmemb; } size_t __wrap_fwrite(const void *ptr, size_t size, size_t nmemb, FILE *fp) -- To view, visit https://review.coreboot.org/c/flashrom/+/58356 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I17d82d281a87129843f547b87c18f52aca23314d Gerrit-Change-Number: 58356 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: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged

1 0

Change in flashrom[master]: tests: Fix fread wrap to return success by default
by Nico Huber (Code Review) 19 Oct '21

19 Oct '21

Attention is currently required from: Edward O'Callaghan, Anastasia Klimchuk. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58356 ) Change subject: tests: Fix fread wrap to return success by default ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/58356 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I17d82d281a87129843f547b87c18f52aca23314d Gerrit-Change-Number: 58356 Gerrit-PatchSet: 2 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: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Tue, 19 Oct 2021 08:09:56 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: tests: Add init-shutdown test for raiden_debug_spi
by Anastasia Klimchuk (Code Review) 19 Oct '21

19 Oct '21

Attention is currently required from: Nico Huber, Thomas Heijligen, Edward O'Callaghan, Angel Pons. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/57918 ) Change subject: tests: Add init-shutdown test for raiden_debug_spi ...................................................................... Patch Set 6: (6 comments) File tests/init_shutdown.c: https://review.coreboot.org/c/flashrom/+/57918/comment/764e64c0_0b646450 PS5, Line 60: **list = calloc(1, sizeof(*list)); > This looks wrong and shouldn't be necessary. The second sizeof() should be […] I just removed all that, and now it works... I honestly don't understand why it works, but it does?! More details: I tried static array, but couldn't make it work, maybe I was missing something obvious. Specifically, when *list was initialised via static array, freeing it failed with "corrupted block", maybe because memory block went out of scope? When I removed free, test failed because it was leaking memory. So it was failing with and without free :\ Leaving comment unresolved, to double-check with you if the code is ok. https://review.coreboot.org/c/flashrom/+/57918/comment/39d42cae_f1f110d5 PS5, Line 122: (void *) > I think anything else would be overkill for mocking. […] Done. https://review.coreboot.org/c/flashrom/+/57918/comment/fc038320_6bf7ec32 PS5, Line 140: sprintf(raiden_debug_param, "address=%s", USB_DEVICE_ADDRESS); > Please use snprintf(). Done https://review.coreboot.org/c/flashrom/+/57918/comment/c69cfe69_01191a6e PS5, Line 140: USB_DEVICE_ADDRESS > Why is the constant a string? Wouldn't it be easier to use %d and a number? For some reason this good idea did not come to my head. I didn't think I can do %d. And now I also can get rid of atoi in libusb_wraps... so good! File tests/io_mock.h: https://review.coreboot.org/c/flashrom/+/57918/comment/efacfa53_2b806244 PS5, Line 43: /* Typedef to make code simpler (no need to type struct all the time). */ > It also makes it oblivious to the reader that these are structs. Maybe […] tests are part of flashrom, so I am removing what is discouraged. https://review.coreboot.org/c/flashrom/+/57918/comment/af4da03f_3bebe1eb PS5, Line 90: ctx > It's not necessary to name every parameter. For instance `ctx` is already […] This is very useful! Lines just got a bit longer because I added few "struct" - and now I can shorten lines back! One thing I left is "list" parameter. It seems to me "list" explains the meaning better, like "libusb device list". -- To view, visit https://review.coreboot.org/c/flashrom/+/57918 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I880a8637ab02de179df9169c1898230bce4dc1c7 Gerrit-Change-Number: 57918 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: 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: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Tue, 19 Oct 2021 07:36:04 +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 init-shutdown test for raiden_debug_spi
by Anastasia Klimchuk (Code Review) 19 Oct '21

19 Oct '21

Attention is currently required from: Thomas Heijligen, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk. Hello build bot (Jenkins), Nico Huber, Thomas Heijligen, Edward O'Callaghan, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/57918 to look at the new patch set (#6). Change subject: tests: Add init-shutdown test for raiden_debug_spi ...................................................................... tests: Add init-shutdown test for raiden_debug_spi This patch adds a test for raiden_debug_spi and lots of libusb wraps. libusb.h becomes required for tests to build and run, since new tests are using libusb structs in depth and opaque symbols not sufficient anymore. BUG=b:181803212 TEST=builds and ninja test Change-Id: I880a8637ab02de179df9169c1898230bce4dc1c7 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M tests/init_shutdown.c M tests/io_mock.h M tests/libusb_wraps.c M tests/meson.build M tests/tests.c M tests/tests.h 6 files changed, 208 insertions(+), 5 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/18/57918/6 -- To view, visit https://review.coreboot.org/c/flashrom/+/57918 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I880a8637ab02de179df9169c1898230bce4dc1c7 Gerrit-Change-Number: 57918 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: 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: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> 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]: tests: Add wraps for all variants of stat
by Anastasia Klimchuk (Code Review) 18 Oct '21

18 Oct '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/+/58103 ) Change subject: tests: Add wraps for all variants of stat ...................................................................... Patch Set 3: (2 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/58103/comment/96fa78a4_1ab89f65 PS2, Line 9: all > When considering non-standard symbol names, there is no "all". People […] Yes you are right, I will do this. Initially I was worried that someone other than me can stumble on this missing wraps, some flashrom developer would have a failing test locally. Don't know how many people are running tests locally though. Keeping the comment unresolved until I gather more info (most likely will do next week). https://review.coreboot.org/c/flashrom/+/58103/comment/ee05a533_607c08b8 PS2, Line 17: TEST=running tests on two different environments, : with 1) stat64 and 2) __xstat64 invoked > this does sound like minijail sort of thing although the change is harmless. […] Yes sure, I will do this. Let's keep this comment unresolved. -- To view, visit https://review.coreboot.org/c/flashrom/+/58103 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I4c5c243acde09dc5bb6b2a14042fcd23a49707db Gerrit-Change-Number: 58103 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-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: Mon, 18 Oct 2021 23:57:22 +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: Fix fread wrap to return success by default
by Anastasia Klimchuk (Code Review) 18 Oct '21

18 Oct '21

Attention is currently required from: Edward O'Callaghan. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58356 ) Change subject: tests: Fix fread wrap to return success by default ...................................................................... Patch Set 2: (1 comment) Patchset: PS2: Rebased and merged with recently submitted changes in tests/ -- To view, visit https://review.coreboot.org/c/flashrom/+/58356 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I17d82d281a87129843f547b87c18f52aca23314d Gerrit-Change-Number: 58356 Gerrit-PatchSet: 2 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: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Mon, 18 Oct 2021 23:34:32 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: tests: Add tests to write on chip
by Anastasia Klimchuk (Code Review) 18 Oct '21

18 Oct '21

Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons. 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/+/58357 to look at the new patch set (#2). Change subject: tests: Add tests to write on chip ...................................................................... tests: Add tests to write on chip This patch adds two tests and initialises page_size in mock chip chip_W25Q128_V. page_size was not needed for previous tests (erase and read). page_size only needed to execute writing on chip with dummyflasher, so it is added here. BUG=b:181803212 TEST=ninja test Change-Id: I6f0336613ab16a7e59857006496e3590ddb14d00 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M tests/chip.c M tests/meson.build M tests/tests.c M tests/tests.h 4 files changed, 72 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/57/58357/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/58357 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I6f0336613ab16a7e59857006496e3590ddb14d00 Gerrit-Change-Number: 58357 Gerrit-PatchSet: 2 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-MessageType: newpatchset

1 0

Change in flashrom[master]: tests: Fix fread wrap to return success by default
by Anastasia Klimchuk (Code Review) 18 Oct '21

18 Oct '21

Attention is currently required from: Edward O'Callaghan, 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/+/58356 to look at the new patch set (#2). Change subject: tests: Fix fread wrap to return success by default ...................................................................... tests: Fix fread wrap to return success by default Successful return value for fread is the number of items read, and default behaviour for all wraps is to return success. This worked previously because all existing tests have custom mocks for fread, so default behaviour hasn't been used. However next patch in this chain adds new test which needs default fread wrap. BUG=b:181803212 TEST=ninja test Change-Id: I17d82d281a87129843f547b87c18f52aca23314d Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M tests/tests.c 1 file changed, 3 insertions(+), 3 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/56/58356/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/58356 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I17d82d281a87129843f547b87c18f52aca23314d Gerrit-Change-Number: 58356 Gerrit-PatchSet: 2 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: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: tests: Add init-shutdown test for raiden_debug_spi
by Nico Huber (Code Review) 18 Oct '21

18 Oct '21

Attention is currently required from: Thomas Heijligen, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/57918 ) Change subject: tests: Add init-shutdown test for raiden_debug_spi ...................................................................... Patch Set 5: (7 comments) File tests/init_shutdown.c: https://review.coreboot.org/c/flashrom/+/57918/comment/ca0cfa38_7a94c60e PS5, Line 60: **list = calloc(1, sizeof(*list)); This looks wrong and shouldn't be necessary. The second sizeof() should be that of a `libusb_device` struct. But as you mention below the device is opaque, so we don't need it. Maybe it helps to imagine it without dynamic allocation. For instance, using static variables: static libusb_device single_dev; static libusb_device *single_entry_list[1] = { [0] = &single_dev }; *list = single_entry_list; **list = &single_dev; /* redundant with the initialization above */ Now, would it make a difference if we'd use `NULL` instead of `&single_dev`? Only if the caller would explicitly for non-NULL, I'd say. (Looking at it now, using a static array would actually work. As long the list is immutable, even multiple calls to libusb_get_device_list() would be ok.) https://review.coreboot.org/c/flashrom/+/57918/comment/2b5b2346_2fa3c8a0 PS5, Line 122: (void *) > I added cast to avoid warnings about discarding const qualifier, hope it's ok. I think anything else would be overkill for mocking. NB. We don't put spaces after casts. https://review.coreboot.org/c/flashrom/+/57918/comment/73eee25d_dcdab415 PS5, Line 140: sprintf(raiden_debug_param, "address=%s", USB_DEVICE_ADDRESS); Please use snprintf(). https://review.coreboot.org/c/flashrom/+/57918/comment/15220623_1b546a17 PS5, Line 140: USB_DEVICE_ADDRESS Why is the constant a string? Wouldn't it be easier to use %d and a number? File tests/io_mock.h: https://review.coreboot.org/c/flashrom/+/57918/comment/069087c4_99bb87b0 PS5, Line 43: /* Typedef to make code simpler (no need to type struct all the time). */ It also makes it oblivious to the reader that these are structs. Maybe that's obvious from the context, though. Generally, it's discouraged in flashrom. I don't mind it in tests/, your call. https://review.coreboot.org/c/flashrom/+/57918/comment/5e366aa6_cb560420 PS5, Line 90: ctx It's not necessary to name every parameter. For instance `ctx` is already obvious from the type so it could just be `libusb_context *,` saying a compatible pointer needs to be passed (doesn't matter how the callee names it). Same for `dev`, `desc` etc. File tests/io_mock.h: https://review.coreboot.org/c/flashrom/+/57918/comment/0882f082_63bb3c5f PS4, Line 42: struct libusb_endpoint_descriptor { : uint8_t offset1[2]; : uint8_t bEndpointAddress; : uint8_t bmAttributes; : uint8_t offset2[5]; : unsigned char *extra; : int extra_length; : }; : typedef struct libusb_endpoint_descriptor libusb_endpoint_descriptor; : : struct libusb_interface_descriptor { : uint8_t offset1[2]; : uint8_t bInterfaceNumber; : uint8_t bAlternateSetting; : uint8_t bNumEndpoints; : uint8_t bInterfaceClass; : uint8_t bInterfaceSubClass; : uint8_t bInterfaceProtocol; : uint8_t offset2; : struct libusb_endpoint_descriptor *endpoint; : }; : typedef struct libusb_interface_descriptor libusb_interface_descriptor; : : struct libusb_interface { : struct libusb_interface_descriptor *altsetting; : int num_altsetting; : }; : typedef struct libusb_interface libusb_interface; : : struct libusb_config_descriptor { : uint8_t offset1[4]; : uint8_t bNumInterfaces; : uint8_t bConfigurationValue; : uint8_t offset2[3]; : struct libusb_interface *interface; : }; : typedef struct libusb_config_descriptor libusb_config_descriptor; : : struct libusb_device {}; : typedef struct libusb_device libusb_device; : : struct libusb_device_descriptor { : uint8_t offset1[8]; : uint16_t idVendor; : uint16_t idProduct; : uint8_t offset2[5]; : uint8_t bNumConfigurations; : }; : typedef struct libusb_device_descriptor libusb_device_descriptor; > I included the header only once in io_mock.h and it seems to work! […] It's already there. In `meson.build`, `flashrom_test_dep` depends on `deps` which has `libusb` added by defalult (set by the `usb` option). Don't ask me if any of it makes sense, but it's there ;) -- To view, visit https://review.coreboot.org/c/flashrom/+/57918 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I880a8637ab02de179df9169c1898230bce4dc1c7 Gerrit-Change-Number: 57918 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: 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: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Mon, 18 Oct 2021 13:44:14 +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 wrap for libusb_init and use it in dediprog test
by Nico Huber (Code Review) 18 Oct '21

18 Oct '21

Nico Huber has submitted this change. ( https://review.coreboot.org/c/flashrom/+/57917 ) Change subject: tests: Add wrap for libusb_init and use it in dediprog test ...................................................................... tests: Add wrap for libusb_init and use it in dediprog test Missing wrap for libusb_init has been discovered while working on the test for raiden_debug_spi. Both dediprog (existing test) and raiden_debug_spi (new test, see later in this chain) are using libusb_init, so it definitely needs to be wrapped and added to io_mock. Why tests worked before, without the wrap: my understanding is that real libusb_init was called for dediprog test? Given that tests definitely should not call any real libusb functions, wrap is needed. BUG=b:181803212 TEST=builds and ninja test Change-Id: I51c9cb96db1afb3298f4d098df96509d3cb3c046 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/57917 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Nico Huber <nico.h(a)gmx.de> --- M tests/init_shutdown.c M tests/io_mock.h M tests/libusb_wraps.c M tests/meson.build 4 files changed, 18 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved diff --git a/tests/init_shutdown.c b/tests/init_shutdown.c index 03e888f..e64e58c 100644 --- a/tests/init_shutdown.c +++ b/tests/init_shutdown.c @@ -54,6 +54,13 @@ #endif } + +int dediprog_libusb_init(void *state, libusb_context **ctx) +{ + *ctx = not_null(); + return 0; +} + int dediprog_libusb_control_transfer(void *state, libusb_device_handle *devh, uint8_t bmRequestType, @@ -75,6 +82,7 @@ { #if CONFIG_DEDIPROG == 1 const struct io_mock dediprog_io = { + .libusb_init = dediprog_libusb_init, .libusb_control_transfer = dediprog_libusb_control_transfer, }; diff --git a/tests/io_mock.h b/tests/io_mock.h index 7c235d8..0bee6ca 100644 --- a/tests/io_mock.h +++ b/tests/io_mock.h @@ -70,6 +70,7 @@ unsigned int (*inl)(void *state, unsigned short port); /* USB I/O */ + int (*libusb_init)(void *state, libusb_context **ctx); int (*libusb_control_transfer)(void *state, libusb_device_handle *devh, uint8_t bmRequestType, diff --git a/tests/libusb_wraps.c b/tests/libusb_wraps.c index 94ef652..978108e 100644 --- a/tests/libusb_wraps.c +++ b/tests/libusb_wraps.c @@ -23,6 +23,14 @@ return not_null(); } +int __wrap_libusb_init(libusb_context **ctx) +{ + LOG_ME; + if (get_io() && get_io()->libusb_init) + return get_io()->libusb_init(get_io()->state, ctx); + return 0; +} + int __wrap_libusb_set_configuration(libusb_device_handle *devh, int config) { LOG_ME; diff --git a/tests/meson.build b/tests/meson.build index c55c5d2..30b1e78 100644 --- a/tests/meson.build +++ b/tests/meson.build @@ -64,6 +64,7 @@ '-Wl,--wrap=test_outl', '-Wl,--wrap=test_inl', '-Wl,--wrap=usb_dev_get_by_vid_pid_number', + '-Wl,--wrap=libusb_init', '-Wl,--wrap=libusb_set_configuration', '-Wl,--wrap=libusb_claim_interface', '-Wl,--wrap=libusb_control_transfer', 4 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. -- To view, visit https://review.coreboot.org/c/flashrom/+/57917 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I51c9cb96db1afb3298f4d098df96509d3cb3c046 Gerrit-Change-Number: 57917 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-MessageType: merged

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit October 2021