Attention is currently required from: Edward O'Callaghan, Anastasia Klimchuk, Evan Benn. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/63903 )
Change subject: libflashrom: Move documentation to header ......................................................................
Patch Set 6:
(10 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/63903/comment/6d157803_6df90e63 PS6, Line 10: FFI What's this?
https://review.coreboot.org/c/flashrom/+/63903/comment/c6439a62_1535d331 PS6, Line 16: and after the change. And what's the result of the test?
File include/libflashrom.h:
https://review.coreboot.org/c/flashrom/+/63903/comment/80ae6f83_e7982ad1 PS6, Line 65: file a file
I'm not sure if the examples are needed.
https://review.coreboot.org/c/flashrom/+/63903/comment/3dc6d0b6_95569b72 PS6, Line 118: /** : * @brief Returns list of supported flash chips : * @return List of supported flash chips, or NULL if an error occurred : */ : struct flashrom_flashchip_info *flashrom_supported_flash_chips(void); : /** : * @brief Returns list of supported mainboards : * @return List of supported mainboards, or NULL if an error occurred : */ : struct flashrom_board_info *flashrom_supported_boards(void); : /** : * @brief Returns list of supported chipsets : * @return List of supported chipsets, or NULL if an error occurred : */ : struct flashrom_chipset_info *flashrom_supported_chipsets(void); Unrelated question, but how does one know the length of the returned list?
https://review.coreboot.org/c/flashrom/+/63903/comment/67b4e967_0a4d0374 PS6, Line 262: short small
https://review.coreboot.org/c/flashrom/+/63903/comment/5143f914_6cb7e45d PS6, Line 356: buffer a buffer
https://review.coreboot.org/c/flashrom/+/63903/comment/aaa7232b_6d9fc741 PS6, Line 372: another I'd use `a` to be more precise, as `flashrom_layout_new()` returns layouts with no regions.
https://review.coreboot.org/c/flashrom/+/63903/comment/30327834_ed495b52 PS6, Line 398: * @param start The start address to be written. : * @param len The length of the region to be written. Maybe @param[out] ?
https://review.coreboot.org/c/flashrom/+/63903/comment/132288c4_2f501720 PS6, Line 416: he I'd avoid using personal pronouns in documentation, the caller isn't necessarily a "he". How about:
The caller must not release the layout as long as it is used through the given flash context.
Here, "it" refers to "the layout".
https://review.coreboot.org/c/flashrom/+/63903/comment/496a5c4f_902d2778 PS6, Line 463: [out] Why is this an out parameter?
-
Angel Pons (Code Review)