Attention is currently required from: Angel Pons, Nikolai Artemiev, Sergii Dmytruk.
14 comments:
File flash.h:
Patch Set #23, Line 203: range
Says less than the type, can be dropped.
Patch Set #23, Line 203: const struct flashrom_wp_chip_config *cfg, uint32_t chip_len
It seems this is exactly the input we need for the `w25` implementation.
But will it suit all future implementations? Should we pass the full flash
definition or even context instead?
Patch Set #23, Line 203: typedef
Why have a typedef for it? All the other function pointers in `struct
flashchip` don't have/need one?
Patch Set #23, Line 203: uint32_t
Please don't hard-code fixed-width types. `unsigned int` should work as
well, `size_t` too, and for most flexibility there is `chipsize_t` in
`layout.h`. I don't promote the latter much because I'm not sure if it's
really necessary. Maybe it would become handy if we'd ever encounter
>4GiB chips (doesn't seem that far away anymore ^^).
Says less than the type already does, can be dropped.
File libflashrom.h:
flashrom_flash_getsize() uses `size_t`? Should this follow?
File writeprotect_ranges.c:
Line break, please.
Not sure if it's worth a TODO. FWIW, this kind of TODO will just stick.
Why 32?
Missing space.
Patch Set #23, Line 34: cfg->bp
Isn't this an implicit pointer? Looks like GCC doesn't warn if compared
to 0. Why does it work? I guess `coeff` overflows and is 0 ;)
Patch Set #23, Line 50: 4 * 1024
4*KiB
Patch Set #23, Line 51: 64 * 1024
64*KiB
Patch Set #23, Line 85: && range->len > 0
Does it matter? i.e. where's the start of an empty range? ^^
To view, visit change 58480. To unsubscribe, or for help writing mail filters, visit settings.