Attention is currently required from: Anastasia Klimchuk, Edward O'Callaghan.
15 comments:
Patchset:
I realise this patch was a intermediate step so the comments are just how it should probably look fo […]
Done
File flashchips.c:
* Temporarily, this file is sorted alphabetically by vendor and name to
* assist with merging the Chromium fork of flashrom.
Can we remove this comment by now? (maybe in a separate patch). […]
Done
Patch Set #5, Line 34: internal_
I agree that "internal" almost always means internal programmer. It is flashromspeak :) […]
Done
Patch Set #5, Line 20844: get_flashchips(void)
Where would the caller get the pointer to flashchip db? and why the caller needs to have the pointer […]
Any code that needs to access the supported flashchips would use this function instead of accessing the array directly. This allows to later load the database of supported flashchips dynamically rather than hard coding it in the binary.
Patch Set #5, Line 20849: unsigned int
maybe so. it was unsigned int before, so i didn't change it. […]
Done
File flashrom.c:
Patch Set #5, Line 1099: return -1;
Maybe you can add `msg_gerr` before returning. […]
Done
File ichspi.c:
const struct flashchip *chip;
struct flashchip *flashchips=get_flashchips();
if (!flashchips)
return NULL;
very clever. and doesn't check the return value. […]
Done
File libflashrom.c:
Patch Set #5, Line 95: unsigned int
shouldn't then i downstairs also be size_t? not against this, but it seems that is a larger consiste […]
Done
File print.c:
Patch Set #5, Line 56: const struct flashchip *chip;
delete
This won't work because we walk the whole array twice and chip is modified the first time.
Patch Set #5, Line 62: flashchips
chip
ditto
Patch Set #5, Line 65: chip = flashchips
delete
ditto.
File print_wiki.c:
Patch Set #5, Line 294: flashchips
get_flashchips()
Given that get_flashchips() might do more in the future, it doesn't make sense to optimize two lines of code saved by making two function calls in my opinion.
Patch Set #5, Line 310: flashchips
get_flashchips()
ditto
File tests/selfcheck.c:
Patch Set #5, Line 80: /* unused */
But it is used two lines below?
Done
flashchips = get_flashchips();
flashchips_size = get_flashchips_size();
Can this be immediately initialised? (and the same above). This is what non-test code is doing.
Done
To view, visit change 75646. To unsubscribe, or for help writing mail filters, visit settings.