On 3/19/24 10:17 PM, Janne Grunau wrote:
Hi,
sorry for the abysmal delay in response.
On Mon, Mar 18, 2024 at 03:17:33PM +0100, Marek Vasut wrote:
On 3/18/24 8:33 AM, Janne Grunau wrote:
+static int usb_blocklist_parse_error(const char *blocklist, size_t pos)
+{
+ printf("usb_blocklist parse error at char %zu in \"%s\"\n", pos,
+ blocklist);
+ return 0;
This could be static void without return 0 at the end.
the return is there to break out of the while loop on parsing errors in a
single statement. This probably won't be necessary after using strsep and
sscanf in the parsing function but see below.
Ahh, now I see it. But then, shouldn't this return -ENODEV here already ?
It returns 0 so that parsing errors in the blocklist do not result
in blocking every USB device. That looked to me like the
less bad error behavior to me.
In unix , 0 is considered success and non-zero failure .
How about this:
- Return -EINVAL here (parsing failed)
If we return 0 / negated errors we do not need this function and we can
simply report the parsing error when usb_device_is_blocked() return
-EINVAL.
- Instead of 'return 1' in usb_device_is_blocked() return -ENODEV
- In usb_select_config(), check
if usb_device_is_blocked returned 0, no device blocked OK
if usb_device_is_blocked returned -ENODEV, device blocked,
return -ENODEV
if usb_device_is_blocked returned any other error, parsing error
return that error
I think the preferable option is to ignore parsing errors. If we
would propagate the error the result would be that every USB device is
ignored.
Good point.
What do you think ?
Fine by me, -EINVAL makes the parsing error reporting less awkward. Only
open question is what should happen on parsing errors.
I agree, ignore/skip the parsing errors and report to user that
something couldn't be parsed, that works.
locally modified and ready to resend once we agree on the behavior on
parsing errors
Thanks, I think we are in agreement.
