On Tue, 28 Jul 2015, Stefan Koch wrote:
> With this patch a flag instead of a variable
> is used for the default device authorization.
>
> Signed-off-by: Stefan Koch <sk...@suse.de>
> ---
> drivers/usb/core/hcd.c | 28 ++++++++++++++++++----------
> drivers/usb/core/usb.c | 2 +-
> include/linux/usb/hcd.h | 6 ++++--
> 3 files changed, 23 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
> index 14357d4..c63d87e 100644
> --- a/drivers/usb/core/hcd.c
> +++ b/drivers/usb/core/hcd.c
> @@ -854,10 +854,11 @@ static ssize_t authorized_default_show(struct device
> *dev,
> {
> struct usb_device *rh_usb_dev = to_usb_device(dev);
> struct usb_bus *usb_bus = rh_usb_dev->bus;
> - struct usb_hcd *usb_hcd;
> + struct usb_hcd *hcd;
>
> - usb_hcd = bus_to_hcd(usb_bus);
> - return snprintf(buf, PAGE_SIZE, "%u\n", usb_hcd->authorized_default);
> + hcd = bus_to_hcd(usb_bus);
> + return snprintf(buf, PAGE_SIZE, "%u\n",
> + HCD_DEV_AUTHORIZED(hcd) ? 1 : 0);
You can do this if you want; it's okay. But there's a common idiom
which is a little shorter: !!HCD_DEV_AUTHORIZED(hcd).
> @@ -868,12 +869,14 @@ static ssize_t authorized_default_store(struct device
> *dev,
> unsigned val;
> struct usb_device *rh_usb_dev = to_usb_device(dev);
> struct usb_bus *usb_bus = rh_usb_dev->bus;
> - struct usb_hcd *usb_hcd;
> + struct usb_hcd *hcd;
>
> - usb_hcd = bus_to_hcd(usb_bus);
> + hcd = bus_to_hcd(usb_bus);
> result = sscanf(buf, "%u\n", &val);
> if (result == 1) {
> - usb_hcd->authorized_default = val ? 1 : 0;
> + val ? set_bit(HCD_FLAG_DEV_AUTHORIZED, &hcd->flags) :
> + clear_bit(HCD_FLAG_DEV_AUTHORIZED, &hcd->flags);
Ugh! Plaese use an "if" statement instead. The "?:" operator should
always be used for its effect (the value it calculates), not its side
effects.
This happens in quite a few places in the patch. It's the sort of
thing one might expect to see in an Obfuscated C program, not in the
kernel.
Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
