Attention is currently required from: Angel Pons, Anastasia Klimchuk.

Patch set 4:Code-Review +1

View Change

5 comments:

• Patchset:

  • Patch Set #4:

    Definitely split this like you suggested but the first test looks great now!

• File tests/chip.c:

  • Patch Set #4, Line 25: int

    `unsigned` unless negative unlocks encode something special?

  • Patch Set #4, Line 52: memcpy(&g_chip_state.buf[start], buf, len);

    validate len does not exceed the actual buffer length. Possibly with a test assert

  • Patch Set #4, Line 59:

    	g_chip_state.unlock_calls++;
    	return 0;
    

    this does depend on the chip I suppose but another possible behavior is the following:

    ```
    g_chip_state.unlock_calls++;
    if (g_chip_state.unlock_calls > 1)
      return -1; // chip already unlocked
    return 0;
    ```

    not sure if that finds anything interesting or is just a wast of time?

  • Patch Set #4, Line 71: memset(&g_chip_state.buf[blockaddr], 0xff, blocklen);

    ditto, ensure blocklen does not exceed the global buffer size.

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