Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk, Sergii Dmytruk.

6 comments:

File libflashrom.h:

Patch Set #9, Line 131:

int flashrom_wp_read_chip_config(const struct flashrom_flashctx *flash, struct flashrom_wp_chip_config *cfg);
int flashrom_wp_write_chip_config(struct flashrom_flashctx *flash, const struct flashrom_wp_chip_config *cfg);
int flashrom_wp_compare_chip_configs(const struct flashrom_wp_chip_config *cfga, const struct flashrom_wp_chip_config *cfgb);

You can omit parameter names if they are self-explanatory from parameter type, which all of these se […]

Done

File writeprotect.c:
- Patch Set #9, Line 27:
```
#define wp_chip_config flashrom_wp_chip_config
#define wp_range flashrom_wp_range
#define wp_mode flashrom_wp_mode
```
  Oh, so why this prefix was added from the very beginning? ;)
  Yep, I'm not sure how much I like it, but it does save a lot of unnecessary in the code.
- Patch Set #9, Line 39:
```
if (spi_read_register(flash, bit.reg, value))
			return 1;
```
  Let's propagate return value, […]
  Done
- Patch Set #9, Line 128: return ret;
  This should be on its own line (and same below).
  Done
- Patch Set #9, Line 158: if (ord == 0) ord = cfga->srp[i] - cfgb->srp[i];
  How does it work? If ord becomes non-0 in the first loop cycle, then the rest of (ord == 0) are skip […]
  If you unroll the loop it looks like this:
  if (ord == 0) ord = cfga->srp[1] - cfgb->srp[1];
  if (ord == 0) ord = cfga->srp[0] - cfgb->srp[0];
  The earlier comparisons are more important so we should only change the value of ord if all previous comparisons gave 0.
- Patch Set #9, Line 161: ord = cfga->cmp - cfgb->cmp;
  Please make it on its own line, and the same for other conditionals below (including ones inside for […]
  Done

To view, visit change 58479. To unsubscribe, or for help writing mail filters, visit settings.