Attention is currently required from: Anastasia Klimchuk, Peter Marheine.

7 comments:

File include/rpmc.h:
- Patch Set #4, Line 71: dependant
```suggestion […]
Done
- Patch Set #4, Line 71: dependant
```suggestion […]
Done
- Patch Set #4, Line 90: struct flashrom_flashctx * flash
  Sorry I just noticed! we usually have asterisk sticking to the variable name (struct flashrom_flashc […]
  Done
File rpmc.c:
- Patch Set #4, Line 131:
```
				if (res != RPMC_SUCCESS) {
					return res;
				}
```
  Found another places here (rpmc_poll_until_finished) where { } not needed because conditional body i […]
  Done
- Patch Set #4, Line 157:
```
	if (keyfile == NULL || read_buf_from_file(key, RPMC_HMAC_KEY_LENGTH, keyfile) != 0) {
		return RPMC_ERROR_KEY_READ;
	}
```
  And here, { } not needed, 1-line body
  Done
- Patch Set #4, Line 497: no error string defined for this value
  Maybe […]
  Done
File sfdp.c:
- Patch Set #4, Line 457:
```
if (sfdp_fill_flash(flash->chip, tbuf, len) == 0) {
					ret = 1;
				}
```
  I have a question: if for some reasons sfdp_fill_flash fails (i.e. returns non-zero), would RPMC feature work? (or any other feature).
  I don't know exactly but from looking at the code it only seems to set information about erase and write calls.
  Maybe this should return error straight away if sfdp_fill_flash fails? Previously it wasn't important because only page 0 of headers was parsed.
  Yes for safety and also consistency I agree.
  Failure to parse the rpmc page is no problem since the feature bit is only set when the function can't fail anymore. I have removed it's influence on the return value of the function.

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