Attention is currently required from: Anastasia Klimchuk, Dmitry Zhadinets.
Peter Marheine has posted comments on this change by Anastasia Klimchuk. ( https://review.coreboot.org/c/flashrom/+/87341?usp=email )
Change subject: libflashrom: Add probing v2 which can find all mathching chips ......................................................................
Patch Set 4:
(2 comments)
File include/libflashrom.h:
https://review.coreboot.org/c/flashrom/+/87341/comment/7c246aab_9dd4cb2c?usp... : PS4, Line 312: const char **all_matched_names, I see from the implementation that the caller is expected to allocate this buffer, but that's not clear from the documentation here and there's currently no guarantee that the buffer is large enough for all the detected chips. Either:
1. add a `size_t` parameter so the caller can indicate how large the buffer is and make the implementation avoid writing more than that many names (while still counting matches if more are found) 2. dynamically allocate the buffer so the caller doesn't need to allocate it and document that it needs to be freed with `flashrom_data_free`. 3. Instead take a callback which the name of matched chips get passed to, allowing the caller to buffer names (or not) as desired (kind of an iterator-based solution).
I'm more inclined toward the second option, since a few allocations shouldn't matter and then a caller never needs to probe multiple times if they discover their buffer was too small.
https://review.coreboot.org/c/flashrom/+/87341/comment/959a938c_66a60e55?usp... : PS4, Line 313: unsigned int *all_matched_count, I'd say it's more conventional to use negative values for error and positive values for success when the logical return value is an int, so you could eliminate this parameter and redefine the return value to be the number of matched chips (if zero of more) or a negative error code.