On Fri, 12 Jun 2015, Stefan Koch wrote: > This is a patch that introduces an interface authorization for USB devices. > The kernel supports already a device authorization bacause of wireless USB. > > But the new interface authorization allows to enable or disable individual interfaces per bitmask > instead allow or deny a whole device. > > The patch is made now much simplier as suggested by Alan Stern. > > Each patch depends on all patches with a lesser number. > > Stefan Koch (5): > usb: Add usb interface authorization: Declare attributes of structures > usb: Add usb interface authorization: Introduces the default interface > authorization > usb: Add usb interface authorization: Control interface probing and > claiming > usb: Add usb interface authorization: Introduces the usb interface > authorization. > usb: Add usb interface authorization: SysFS part of usb interface > authorization. There is a lot of questionable material here. First of all, I agree with Krzysztof that having an "authorized" attribute in each interface's sysfs directory would be simpler and easier to use than having a bitmask of all authorized interfaces. Secondly, the patches have not been carefully edited. There are several misspelled words in comments and descriptions. And why does patch 3/5 modify drivers/base/base.h and include/linux/device.h? Thirdly, what is the purpose of the mask_changed bit? The changelog describes it as "a status bit to control a manual setting of the mask", which is not very clear. _How_ does it control manual setting of the mask? _Why_ does manual setting of the mask need to be controlled? Fourthly, it doesn't look like usb_set_configuration() does the right thing. It should call usb_enable_interface() whether the interface is authorized or not. Fifthly, the code style is awkward in several places. For instance, it looks silly to do this (usb_alloc_dev in patch 2/5): dev->mask = 0; /* invert the mask. each bit of the mask is now TRUE. * all interfaces should be allowed. */ dev->mask = ~dev->mask; Not to mention that the accepted style for multi-line comments is like this: /* * This is a * long comment. */ Also, the kernel doesn't use CamelCase names like intfNr. All these issues will have to be fixed before the patches can be accepted. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
