[PATCH v2 4/6] usb: Add environment based device blocklist

[Janne Grunau]

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. 
 
> 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.

locally modified and ready to resend once we agree on the behavior on
parsing errors

Janne