Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/37406 )
Change subject: dediprog: add serial argument ......................................................................
Patch Set 2:
(6 comments)
https://review.coreboot.org/c/flashrom/+/37406/2/dediprog.c File dediprog.c:
https://review.coreboot.org/c/flashrom/+/37406/2/dediprog.c@1008 PS2, Line 1008: * @index index of the USB device : * @serial_number serial number of the USB device (id is ignored then) : * @return 0 for success, -1 for error, -2 for busy device Please re-align.
https://review.coreboot.org/c/flashrom/+/37406/2/dediprog.c@1065 PS2, Line 1065: *serial_number; Line break seems unnecessary.
https://review.coreboot.org/c/flashrom/+/37406/2/dediprog.c@1099 PS2, Line 1099: serial_number = extract_programmer_param("serial"); Leaking `serial_number` and missing check if conflicting options (id, device) are given.
One way would be to do the check in advance:
serial_number = ... id_str = ... device = ... if (serial_number && id_str || serial_number && device || id_str && device) { msg_perr("Error: Only one of 'serial', 'id' or 'device' can be used.\n"); // bail out? }
https://review.coreboot.org/c/flashrom/+/37406/2/dediprog.c@1192 PS2, Line 1192: if (serial_number) { : if (dediprog_open(0, serial_number)) { : return 1; : } : found_id = dediprog_read_id(); Can be handled more elegantly in the else path below.
https://review.coreboot.org/c/flashrom/+/37406/2/dediprog.c@1232 PS2, Line 1232: NULL Could just pass `serial_number` here. Either one of the variables would be 0.
https://review.coreboot.org/c/flashrom/+/37406/2/dediprog.c@1236 PS2, Line 1236: } From here, we won't need `serial_number` anymore.