Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons.
6 comments:
Patchset:
> 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:
Definitely split this like you suggested but the first test looks great now!
thanks!
File tests/chip.c:
`unsigned` unless negative unlocks encode something special?
Done
Patch Set #4, Line 52: memcpy(&g_chip_state.buf[start], buf, len);
validate len does not exceed the actual buffer length. […]
Done
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.
Patch Set #4, Line 71: memset(&g_chip_state.buf[blockaddr], 0xff, blocklen);
ditto, ensure blocklen does not exceed the global buffer size.
Done
To view, visit change 56501. To unsubscribe, or for help writing mail filters, visit settings.