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

Patch set 1:Code-Review +1

View Change

3 comments:

• File tests/chip.c:

  • Patch Set #1, Line 29: g_chip_state

    I wonder if it is time to embed this into `data` of the flashctx to avoid possible test coupling. Things like `reset_global_chip_state()` are signs that this could be a good idea to consider.

  • Patch Set #1, Line 89:

    static void reset_global_chip_state()
    {
    	g_chip_state.unlock_calls = 0;
    }
    

    Just inline this function to the one call site.

    OK more generally in OOP speak here; It is best to avoid having global state management functions which are akin to a singleton class and instead use generators for this kind of state.

  • Patch Set #1, Line 94: setup_for_chip

    Usually this would be a statement, e.g. just `set_up_chip`.

    +1 drop the _for_ just `setup_chip()`

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