Change in flashrom[master]: usbdev: Refactor device discovery code

[Daniel Thompson (Code Review)]

Daniel Thompson has posted comments on this change. ( https://review.coreboot.org/27444 )

Change subject: usbdev: Refactor device discovery code
......................................................................


Patch Set 2:

(5 comments)

Thanks for the review.

https://review.coreboot.org/#/c/27444/2/usbdev.c
File usbdev.c:

https://review.coreboot.org/#/c/27444/2/usbdev.c@25
PS2, Line 25: _t
> _t is for standard libraries, _func maybe? Actually, as it's only […]
> It is only used once but the prototype of get_by_vid_pid_filter() becomes tough to read without the typedef. I'll switch this to _func.

https://review.coreboot.org/#/c/27444/2/usbdev.c@84
PS2, Line 84: void *
> `const void *` to make that clear (unless the compiler won't let us)
> Ack

https://review.coreboot.org/#/c/27444/2/usbdev.c@105
PS2, Line 105: 	/* no need to NULL check handle; the conditional decrement makes it unnecessary */
> No idea what this is supposed to mean. If you need to read the code […]
> Fair point. I'll also use the typedef to describe better how the filter works (specifically the way we call it with handle == NULL as a pre-check and handle set as a post-check).

As it happens this comments is saying that if handle is non-null then it is guaranteed that *nump is false. However its probably clearer just to add a check of the handle than to explain why no check is needed.


https://review.coreboot.org/#/c/27444/2/usbdev.c@118
PS2, Line 118:
> no space after closing parenthesis
> Ack

https://review.coreboot.org/#/c/27444/2/usbdev.c@115
PS2, Line 115: struct libusb_device_handle *usb_dev_get_by_vid_pid_serial(
             : 		struct libusb_context *usb_ctx, uint16_t vid, uint16_t pid, const char *serialno)
             : {
             : 	return get_by_vid_pid_filter(usb_ctx, vid, pid, filter_by_serial, (void *) serialno);
             : }
> please group with the used filter function
> Ack