On, Sun, 12 Aug 2018 11:34:59 -0700 Joe Perches wrote: >On Sun, 2018-08-12 at 19:06 +0100, Tom Todd wrote: >> Fixed a code styling issue >while it's OK to fix style only issues, it's much better >to reorganize the code for reader clarity. Ok, thank you, I've taken your suggestions and created the a new version Restructured method proc_ioctl for readability and fixed code style errors. Signed-off-by: Tom Todd <thomas.m.a.todd@xxxxxxxxx> --- drivers/usb/core/devio.c | 67 ++++++++++++++++++++++------------------ 1 file changed, 37 insertions(+), 30 deletions(-) diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c index 476dcc5f2da3..740e60e086e2 100644 --- a/drivers/usb/core/devio.c +++ b/drivers/usb/core/devio.c @@ -2117,46 +2117,52 @@ static int proc_ioctl(struct usb_dev_state *ps, struct usbdevfs_ioctl *ctl) /* alloc buffer */ size = _IOC_SIZE(ctl->ioctl_code); if (size > 0) { - buf = kmalloc(size, GFP_KERNEL); - if (buf == NULL) - return -ENOMEM; - if ((_IOC_DIR(ctl->ioctl_code) & _IOC_WRITE)) { - if (copy_from_user(buf, ctl->data, size)) { - kfree(buf); + if (_IOC_DIR(ctl->ioctl_code) & _IOC_WRITE) { + buf = memdup_user(ctl->data, size); + if (!buf) return -EFAULT; - } } else { - memset(buf, 0, size); + buf = kmalloc(size, GFP_KERNEL); + if (!buf) + return -ENOMEM; } } if (!connected(ps)) { - kfree(buf); - return -ENODEV; + retval = -ENODEV; + goto exit; } - if (ps->dev->state != USB_STATE_CONFIGURED) + if (ps->dev->state != USB_STATE_CONFIGURED) { retval = -EHOSTUNREACH; - else if (!(intf = usb_ifnum_to_if(ps->dev, ctl->ifno))) + goto exit; + } + + intf = usb_ifnum_to_if(ps->dev, ctl->ifno); + if (!intf) { retval = -EINVAL; - else switch (ctl->ioctl_code) { + goto exit; + } + switch (ctl->ioctl_code) { /* disconnect kernel driver from interface */ case USBDEVFS_DISCONNECT: - if (intf->dev.driver) { - driver = to_usb_driver(intf->dev.driver); - dev_dbg(&intf->dev, "disconnect by usbfs\n"); - usb_driver_release_interface(driver, intf); - } else + if (!intf->dev.driver) { retval = -ENODATA; + goto exit; + } + driver = to_usb_driver(intf->dev.driver); + dev_dbg(&intf->dev, "disconnect by usbfs\n"); + usb_driver_release_interface(driver, intf); break; /* let kernel drivers try to (re)bind to the interface */ case USBDEVFS_CONNECT: - if (!intf->dev.driver) - retval = device_attach(&intf->dev); - else + if (!intf->dev.driver) { retval = -EBUSY; + goto exit; + } + retval = device_attach(&intf->dev); break; /* talk directly to the interface's driver */ @@ -2165,20 +2171,21 @@ static int proc_ioctl(struct usb_dev_state *ps, struct usbdevfs_ioctl *ctl) driver = to_usb_driver(intf->dev.driver); if (driver == NULL || driver->unlocked_ioctl == NULL) { retval = -ENOTTY; - } else { - retval = driver->unlocked_ioctl(intf, ctl->ioctl_code, buf); - if (retval == -ENOIOCTLCMD) - retval = -ENOTTY; + goto exit; } + retval = driver->unlocked_ioctl(intf, ctl->ioctl_code, buf); + if (retval == -ENOIOCTLCMD) { + retval = -ENOTTY; + goto exit; + } + break; } /* cleanup and return */ - if (retval >= 0 - && (_IOC_DIR(ctl->ioctl_code) & _IOC_READ) != 0 - && size > 0 - && copy_to_user(ctl->data, buf, size) != 0) + if (retval >= 0 && (_IOC_DIR(ctl->ioctl_code) & _IOC_READ) != 0 && + size > 0 && copy_to_user(ctl->data, buf, size) != 0) retval = -EFAULT; - +exit: kfree(buf); return retval; } -- 2.18.0