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".