Attention is currently required from: Felix Singer, Thomas Heijligen, Anastasia Klimchuk. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/63488 )
Change subject: ich_descriptors_tool: Fix -Wsign-compare warnings ......................................................................
Patch Set 1:
(2 comments)
File util/ich_descriptors_tool/ich_descriptors_tool.c:
https://review.coreboot.org/c/flashrom/+/63488/comment/f87f8527_2a7692ad PS1, Line 87: uint32_t
I would prefer doing variable declarations at the beginning of the method, so that these are not sca […]
If this is the `write()` I'm thinking of, it returns a `ssize_t`: https://man7.org/linux/man-pages/man2/write.2.html
Also, using a fixed-width type (uint32_t) here is unnecessary, as there are no width constraints.
To avoid signed/unsigned comparisons, I'd do the following:
ssize_t ret = write(fh, &dump[base >> 2], file_len); if (ret < 0 || (size_t)ret != file_len) {
The `size_t` cast is safe because it is only performed when `!(ret < 0)`, i.e. `ret >= 0`.
https://review.coreboot.org/c/flashrom/+/63488/comment/4608c7d5_35c9cdfc PS1, Line 270: uint32_t There's no need to use a fixed-width type here. Please use `size_t` instead.
-
Angel Pons (Code Review)