Attention is currently required from: Edward O'Callaghan, Angel Pons, Nikolai Artemiev, Sergii Dmytruk.
13 comments:
File libflashrom.h:
Patch Set #23, Line 137: #define FLASHROM_WP_MAX_RANGES 128
Should we use dynamic allocation instead?
Patch Set #23, Line 143: size_t
Technically `size_t` is supposed to specify a byte count. It's often
accepted as a short hand for unsigned int, but I don't think we should
do that in the public API.
File writeprotect.c:
That this is actually useful (to search for a configuration) is only
visible in a later commit. Please mention such things in the commit
message.
Patch Set #23, Line 187: preferred
What is preferred and why?
Patch Set #23, Line 233: TB in a OTP register
Please mention the chip where you have seen something like this. Sometimes
these things are hard to believe and you can spare future readers the pain
of having to search for such a chip.
Patch Set #23, Line 234: perserved
p*re*served
ARRAY_SIZE(bits->bp) + 1 /* TB */ + 1 /* SEC */ + 1 /* CMP */ ?
Patch Set #23, Line 240: can_write_bit(bits->bp[i])
This would stop at the first not writable bit and skip further writable
ones, right?
Patch Set #23, Line 254: (1 << bit_count)
Nit, no parentheses needed.
Why 32?
Patch Set #23, Line 255: for (uint32_t range_index = 0; range_index < *count; range_index++) {
Please check that the destination arrays don't overflow (wouldn't
be necessary if they were allocated with `*count` elements).
Not sure if we should use tabs here.
Patch Set #23, Line 308: struct wp_range_cfg_pair range_cfg_pairs[FLASHROM_WP_MAX_RANGES];
Not on the stack, please. Flashrom sometimes runs in embedded environments.
To view, visit change 58481. To unsubscribe, or for help writing mail filters, visit settings.