Attention is currently required from: Peter Marheine.

Patch set 2:Code-Review +1

View Change

5 comments:

• File lspcon_i2c_spi.c:

  • Patch Set #1, Line 486: msg_perr("%s: one of devpath or bus must be specified\n", __func__);

    Revised further so invalid bus values are distinct from unspecified, allowing the two options to be […]

    Looks pretty good, thanks!

• File lspcon_i2c_spi.c:

  • Patch Set #2, Line 457: __func__);

    nit: `__func__` probably fits on the previous line

  • Patch Set #2, Line 463: __func__);

    nit: `__func__` most likely fits on the previous line

  • Patch Set #2, Line 489:

    nit: replace 8 spaces with a tab

  • Patch Set #2, Line 492: } else if (device_path != NULL) {

    The first two cases are error paths that return an error, whereas the last two cases are the "normal" paths. I wouldn't use `else` with the former, in order to differentiate both kinds of paths:

     if (device_path != NULL && bus_number >= 0) {
     	msg_perr("%s: only one of bus and devpath may be specified\n", __func__);
     	free(device_path);
     	return SPI_GENERIC_ERROR;
     }
     if (device_path == NULL && bus_number < 0) {
     	msg_perr("%s: one of bus and devpath must be specified\n", __func__);
     	return SPI_GENERIC_ERROR;
     }
     if (device_path != NULL) {
     	fd = i2c_open_path(device_path, REGISTER_ADDRESS, 0);
     	free(device_path);
     } else {
     	fd = i2c_open(bus_number, REGISTER_ADDRESS, 0);
     }

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