Attention is currently required from: Anastasia Klimchuk, Dmitry Zhadinets.

View Change

2 comments:

• File include/libflashrom.h:

  • Patch Set #4, 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.

  • Patch Set #4, 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.

To view, visit change 87341. To unsubscribe, or for help writing mail filters, visit settings.