Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/25683 )
Change subject: Add support for National Instruments USB-845x devices ......................................................................
Patch Set 26: Code-Review+1
(2 comments)
Looks good, thanks for the update. Alas, I seem to have found an issue that I missed earlier.
https://review.coreboot.org/c/flashrom/+/25683/26/ni845x_spi.c File ni845x_spi.c:
https://review.coreboot.org/c/flashrom/+/25683/26/ni845x_spi.c@249 PS26, Line 249: i >= 0 Did you test this with unexpected inputs, e.g. command line argument of 1.0V?
Didn't see this earlier, but it looks like we'd end up with `i = -1`. But the code after this loops assumes that we stop at 0 (i.e. `i` is always a valid array index). So the last iteration should be for `i == 1`, i.e. the condition should be `i > 0`.
https://review.coreboot.org/c/flashrom/+/25683/26/ni845x_spi.c@440 PS26, Line 440: strlen(ignore_io_voltage_limits_str) == 3 : && strstr("yes", ignore_io_voltage_limits_str) == 0 This should be the same as
strcmp(ignore_io_voltage_limits_str, "yes") == 0
no?
-
Nico Huber (Code Review)