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

Patch set 3:Code-Review +1

3 comments:

File buspirate_spi.c:
- Patch Set #3, Line 58: unsigned char **bp_commbuf
  I was considering the idea to pass a pointer to `struct bp_spi_data` into buspirate_commbuf_grow but […]
  Well, it's roughly the same either way. Local variables and bp_data need to be
  synchronized after a change. So after a call to buspirate_commbuf_grow(), it's
  either
```
    bp_commbuf = bp_data->bp_commbuf;
```
  or
```
    bp_data->bp_commbuf = bp_commbuf;
```
  As `bp_commbufsize` is only consumed in buspirate_commbuf_grow() we could even
  save us a local variable for it when passing `bp_data`. We'd have to switch
  allocation of `bp_commbuf` and `bp_data`, though.
  This can all change later, however. This patch looks good already.
- Patch Set #3, Line 329:
```
	unsigned char *bp_commbuf = NULL;
	int bp_commbufsize = 0;
```
  Nit, `NULL`/`0` should never be consumed, so the initializations are unnecessary.
- Patch Set #3, Line 695:
```
	/* Storing the latest values into data, commbuf migth have grown during init. */
	bp_data->bp_commbuf = bp_commbuf;
	bp_data->bp_commbufsize = bp_commbufsize;
```
  In the light of the discussion above, this shouldn't be needed?

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