Attention is currently required from: Thomas Heijligen, Angel Pons, qianfan, Anastasia Klimchuk, Nicholas Chin.

View Change

11 comments:

• Patchset:

  • Patch Set #4:

    Thank you so much for your work! I have just few comments.

• File ch347_spi.c:

  • Patch Set #4, Line 55:

    struct ch347_spi_data {
    	struct libusb_device_handle *handle;
    };
    

    Place this declaration before `devs_ch347_spi` variable.

  • Patch Set #4, Line 62:

    if (ch347_data->handle == NULL)
    		return -1;
    

    drop condition

  • Patch Set #4, Line 72: ch347_data->handle = NULL;

    in the next line of the code, we do `free(data)`, so there's no reason to care about this pointer

  • Patch Set #4, Line 109: uint8_t buffer[CH347_PACKET_SIZE] = {0};

    How about moving this variable to the rest of data? So the code will be after the data.

  • Patch Set #4, Line 265:

    struct libusb_device_handle *handle = NULL;
    	struct ch347_spi_data *ch347_data = calloc(1, sizeof(*ch347_data));
    

    As far as you allocate `ch347_spi_data` at the beginning of initialization, you can drop `struct libusb_device_handle *handle` variable.

  • Patch Set #4, Line 269: -1

    Please, use `1` value just to be consistency with other initialization functions.

  • Patch Set #4, Line 275: -1

    Same here, `1`

  • Patch Set #4, Line 287: handle

    ch347_data->handle = ...;

  • Patch Set #4, Line 290: -1

    Same here, `1`

  • Patch Set #4, Line 335: -1

    Same here, `1`

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