2015-02-07 21:57:55

by Bas Peters

[permalink] [raw]
Subject: [PATCH 0/6] drivers: usb: core: fix various checkpatch errors.

This patchset adresses various checkpatch errors found when running the
checkpatch script on the directory.

Bas Peters (6):
drivers: usb: core: devio.c: remove assignment of variables in if
conditions.
drivers: usb: core: devio.c: fix whitespace errors thrown by
checkpatch.pl
drivers: usb: core: hcd.c: remove assignment of variables in if
conditions.
drivers: usb: core: hub.c: remove NULL initialization of static
variables.
drivers: usb: core: hub.c: remove assignment of variables in if
conditions.
drivers: usb: core: endpoint.c: fix trivial whitespace issue

drivers/usb/core/devio.c | 76 ++++++++++++++++++++++++---------------------
drivers/usb/core/endpoint.c | 2 +-
drivers/usb/core/hcd.c | 15 ++++++---
drivers/usb/core/hub.c | 11 ++++---
4 files changed, 58 insertions(+), 46 deletions(-)

--
2.1.0


2015-02-07 21:57:59

by Bas Peters

[permalink] [raw]
Subject: [PATCH 1/6] drivers: usb: core: devio.c: remove assignment of variables in if conditions.

This patch removes assignment of variables in if conditions in
accordance witht the CodingStyle.

Signed-off-by: Bas Peters <[email protected]>
---
drivers/usb/core/devio.c | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
index 0b59731..ea3c737 100644
--- a/drivers/usb/core/devio.c
+++ b/drivers/usb/core/devio.c
@@ -1081,7 +1081,8 @@ static int proc_bulk(struct usb_dev_state *ps, void __user *arg)
ret = usbfs_increase_memory_usage(len1 + sizeof(struct urb));
if (ret)
return ret;
- if (!(tbuf = kmalloc(len1, GFP_KERNEL))) {
+ tbuf = kmalloc(len1, GFP_KERNEL);
+ if (!tbuf) {
ret = -ENOMEM;
goto done;
}
@@ -1223,7 +1224,8 @@ static int proc_setintf(struct usb_dev_state *ps, void __user *arg)

if (copy_from_user(&setintf, arg, sizeof(setintf)))
return -EFAULT;
- if ((ret = checkintf(ps, setintf.interface)))
+ ret = checkintf(ps, setintf.interface);
+ if (ret)
return ret;

destroy_async_on_interface(ps, setintf.interface);
@@ -1392,7 +1394,8 @@ static int proc_do_submiturb(struct usb_dev_state *ps, struct usbdevfs_urb *uurb
number_of_packets = uurb->number_of_packets;
isofrmlen = sizeof(struct usbdevfs_iso_packet_desc) *
number_of_packets;
- if (!(isopkt = kmalloc(isofrmlen, GFP_KERNEL)))
+ isopkt = kmalloc(isofrmlen, GFP_KERNEL);
+ if (!isopkt)
return -ENOMEM;
if (copy_from_user(isopkt, iso_frame_desc, isofrmlen)) {
ret = -EFAULT;
@@ -1901,7 +1904,8 @@ static int proc_releaseinterface(struct usb_dev_state *ps, void __user *arg)

if (get_user(ifnum, (unsigned int __user *)arg))
return -EFAULT;
- if ((ret = releaseintf(ps, ifnum)) < 0)
+ ret = releaseintf(ps, ifnum);
+ if (ret < 0)
return ret;
destroy_async_on_interface (ps, ifnum);
return 0;
@@ -1916,7 +1920,8 @@ static int proc_ioctl(struct usb_dev_state *ps, struct usbdevfs_ioctl *ctl)
struct usb_driver *driver = NULL;

/* alloc buffer */
- if ((size = _IOC_SIZE(ctl->ioctl_code)) > 0) {
+ size = _IOC_SIZE(ctl->ioctl_code);
+ if (size > 0) {
buf = kmalloc(size, GFP_KERNEL);
if (buf == NULL)
return -ENOMEM;
--
2.1.0

2015-02-07 21:59:31

by Bas Peters

[permalink] [raw]
Subject: [PATCH 2/6] drivers: usb: core: devio.c: fix whitespace errors thrown by checkpatch.pl

This patch fixes errors generated by checkpatch.pl relating to
whitespace issues.

Signed-off-by: Bas Peters <[email protected]>
---
drivers/usb/core/devio.c | 61 ++++++++++++++++++++++++------------------------
1 file changed, 31 insertions(+), 30 deletions(-)

diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
index ea3c737..7c932d9 100644
--- a/drivers/usb/core/devio.c
+++ b/drivers/usb/core/devio.c
@@ -103,7 +103,7 @@ MODULE_PARM_DESC(usbfs_snoop, "true to log all usbfs traffic");
#define snoop(dev, format, arg...) \
do { \
if (usbfs_snoop) \
- dev_info(dev , format , ## arg); \
+ dev_info(dev, format, ## arg); \
} while (0)

enum snoop_when {
@@ -1320,7 +1320,7 @@ static int proc_do_submiturb(struct usb_dev_state *ps, struct usbdevfs_urb *uurb
is_in = (uurb->endpoint & USB_ENDPOINT_DIR_MASK) != 0;

u = 0;
- switch(uurb->type) {
+ switch (uurb->type) {
case USBDEVFS_URB_TYPE_CONTROL:
if (!usb_endpoint_xfer_control(&ep->desc))
return -EINVAL;
@@ -1944,38 +1944,39 @@ static int proc_ioctl(struct usb_dev_state *ps, struct usbdevfs_ioctl *ctl)
retval = -EHOSTUNREACH;
else if (!(intf = usb_ifnum_to_if(ps->dev, ctl->ifno)))
retval = -EINVAL;
- else 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
- retval = -ENODATA;
- break;
+ else
+ 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
+ retval = -ENODATA;
+ break;

- /* let kernel drivers try to (re)bind to the interface */
- case USBDEVFS_CONNECT:
- if (!intf->dev.driver)
- retval = device_attach(&intf->dev);
- else
- retval = -EBUSY;
- break;
+ /* let kernel drivers try to (re)bind to the interface */
+ case USBDEVFS_CONNECT:
+ if (!intf->dev.driver)
+ retval = device_attach(&intf->dev);
+ else
+ retval = -EBUSY;
+ break;

- /* talk directly to the interface's driver */
- default:
- if (intf->dev.driver)
- 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)
+ /* talk directly to the interface's driver */
+ default:
+ if (intf->dev.driver)
+ 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;
+ }
}
- }

/* cleanup and return */
if (retval >= 0
--
2.1.0

2015-02-07 21:58:06

by Bas Peters

[permalink] [raw]
Subject: [PATCH 3/6] drivers: usb: core: hcd.c: remove assignment of variables in if conditions.

This patch removes assignment of variables in if conditions,
as specified in CodingStyle.

Signed-off-by: Bas Peters <[email protected]>
---
drivers/usb/core/hcd.c | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 11cee55..37c40d1 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -2683,15 +2683,18 @@ int usb_add_hcd(struct usb_hcd *hcd,
* bottom up so that hcds can customize the root hubs before hub_wq
* starts talking to them. (Note, bus id is assigned early too.)
*/
- if ((retval = hcd_buffer_create(hcd)) != 0) {
+ retval = hcd_buffer_create(hcd);
+ if (retval != 0) {
dev_dbg(hcd->self.controller, "pool alloc failed\n");
goto err_create_buf;
}

- if ((retval = usb_register_bus(&hcd->self)) < 0)
+ retval = usb_register_bus(&hcd->self);
+ if (retval < 0)
goto err_register_bus;

- if ((rhdev = usb_alloc_dev(NULL, &hcd->self, 0)) == NULL) {
+ rhdev = usb_alloc_dev(NULL, &hcd->self, 0);
+ if (rhdev == NULL) {
dev_err(hcd->self.controller, "unable to allocate root hub\n");
retval = -ENOMEM;
goto err_allocate_root_hub;
@@ -2733,7 +2736,8 @@ int usb_add_hcd(struct usb_hcd *hcd,
/* "reset" is misnamed; its role is now one-time init. the controller
* should already have been reset (and boot firmware kicked off etc).
*/
- if (hcd->driver->reset && (retval = hcd->driver->reset(hcd)) < 0) {
+ retval = hcd->driver->reset(hcd);
+ if (hcd->driver->reset && retval < 0) {
dev_err(hcd->self.controller, "can't setup: %d\n", retval);
goto err_hcd_driver_setup;
}
@@ -2765,7 +2769,8 @@ int usb_add_hcd(struct usb_hcd *hcd,
}

/* starting here, usbcore will pay attention to this root hub */
- if ((retval = register_root_hub(hcd)) != 0)
+ retval = register_root_hub(hcd);
+ if (retval != 0)
goto err_register_root_hub;

retval = sysfs_create_group(&rhdev->dev.kobj, &usb_bus_attr_group);
--
2.1.0

2015-02-07 21:58:59

by Bas Peters

[permalink] [raw]
Subject: [PATCH 4/6] drivers: usb: core: hub.c: remove NULL initialization of static variables.

NULL initialization of static variables is unnecessary as GCC kindly does
this for us.

Signed-off-by: Bas Peters <[email protected]>
---
drivers/usb/core/hub.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index aeb50bb..82983d9 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -49,7 +49,7 @@ static void hub_event(struct work_struct *work);
DEFINE_MUTEX(usb_port_peer_mutex);

/* cycle leds on hubs that aren't blinking for attention */
-static bool blinkenlights = 0;
+static bool blinkenlights;
module_param (blinkenlights, bool, S_IRUGO);
MODULE_PARM_DESC (blinkenlights, "true to cycle leds on hubs");

@@ -78,7 +78,7 @@ MODULE_PARM_DESC(initial_descriptor_timeout,
* otherwise the new scheme is used. If that fails and "use_both_schemes"
* is set, then the driver will make another attempt, using the other scheme.
*/
-static bool old_scheme_first = 0;
+static bool old_scheme_first;
module_param(old_scheme_first, bool, S_IRUGO | S_IWUSR);
MODULE_PARM_DESC(old_scheme_first,
"start with the old device initialization scheme");
--
2.1.0

2015-02-07 21:58:09

by Bas Peters

[permalink] [raw]
Subject: [PATCH 5/6] drivers: usb: core: hub.c: remove assignment of variables in if conditions.

As specified in the CodingStyle.

Signed-off-by: Bas Peters <[email protected]>
---
drivers/usb/core/hub.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 82983d9..9afe8b0 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -671,8 +671,8 @@ resubmit:
if (hub->quiescing)
return;

- if ((status = usb_submit_urb (hub->urb, GFP_ATOMIC)) != 0
- && status != -ENODEV && status != -EPERM)
+ status = usb_submit_urb (hub->urb, GFP_ATOMIC);
+ if (status != 0 && status != -ENODEV && status != -EPERM)
dev_err (hub->intfdev, "resubmit --> %d\n", status);
}

@@ -795,7 +795,8 @@ int usb_hub_clear_tt_buffer(struct urb *urb)
* since each TT has "at least two" buffers that can need it (and
* there can be many TTs per hub). even if they're uncommon.
*/
- if ((clear = kmalloc (sizeof *clear, GFP_ATOMIC)) == NULL) {
+ clear = kmalloc (sizeof *clear, GFP_ATOMIC);
+ if (clear == NULL) {
dev_err (&udev->dev, "can't save CLEAR_TT_BUFFER state\n");
/* FIXME recover somehow ... RESET_TT? */
return -ENOMEM;
--
2.1.0

2015-02-07 21:58:17

by Bas Peters

[permalink] [raw]
Subject: [PATCH 6/6] drivers: usb: core: endpoint.c: fix trivial whitespace issue

Changes space-based indentation to tab-based indentation.

Signed-off-by: Bas Peters <[email protected]>
---
drivers/usb/core/endpoint.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/core/endpoint.c b/drivers/usb/core/endpoint.c
index 39a2402..101983b 100644
--- a/drivers/usb/core/endpoint.c
+++ b/drivers/usb/core/endpoint.c
@@ -51,7 +51,7 @@ static ssize_t wMaxPacketSize_show(struct device *dev,
{
struct ep_device *ep = to_ep_device(dev);
return sprintf(buf, "%04x\n",
- usb_endpoint_maxp(ep->desc) & 0x07ff);
+ usb_endpoint_maxp(ep->desc) & 0x07ff);
}
static DEVICE_ATTR_RO(wMaxPacketSize);

--
2.1.0

2015-02-08 11:15:26

by Sergei Shtylyov

[permalink] [raw]
Subject: Re: [PATCH 3/6] drivers: usb: core: hcd.c: remove assignment of variables in if conditions.

Hello.

On 2/8/2015 12:55 AM, Bas Peters wrote:

> This patch removes assignment of variables in if conditions,
> as specified in CodingStyle.

> Signed-off-by: Bas Peters <[email protected]>
> ---
> drivers/usb/core/hcd.c | 15 ++++++++++-----
> 1 file changed, 10 insertions(+), 5 deletions(-)

> diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
> index 11cee55..37c40d1 100644
> --- a/drivers/usb/core/hcd.c
> +++ b/drivers/usb/core/hcd.c
[...]
> @@ -2733,7 +2736,8 @@ int usb_add_hcd(struct usb_hcd *hcd,
> /* "reset" is misnamed; its role is now one-time init. the controller
> * should already have been reset (and boot firmware kicked off etc).
> */
> - if (hcd->driver->reset && (retval = hcd->driver->reset(hcd)) < 0) {
> + retval = hcd->driver->reset(hcd);

This will crash if 'hcd->driver->reset' is NULL (which is only checked below).

> + if (hcd->driver->reset && retval < 0) {

It wasn't equivalent change anyway as the right part of && is only
executed if the left part is true.

WBR, Sergei

2015-02-08 11:17:34

by Sergei Shtylyov

[permalink] [raw]
Subject: Re: [PATCH 4/6] drivers: usb: core: hub.c: remove NULL initialization of static variables.

On 2/8/2015 12:55 AM, Bas Peters wrote:

> NULL

Rather 0-.

> initialization of static variables is unnecessary as GCC kindly does
> this for us.

It's rather the C run-time library that does this.

> Signed-off-by: Bas Peters <[email protected]>

[...]

WBR, Sergei

2015-02-08 11:20:09

by Sergei Shtylyov

[permalink] [raw]
Subject: Re: [PATCH 5/6] drivers: usb: core: hub.c: remove assignment of variables in if conditions.

On 2/8/2015 12:55 AM, Bas Peters wrote:

> As specified in the CodingStyle.

> Signed-off-by: Bas Peters <[email protected]>
> ---
> drivers/usb/core/hub.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)

> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index 82983d9..9afe8b0 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -671,8 +671,8 @@ resubmit:
> if (hub->quiescing)
> return;
>
> - if ((status = usb_submit_urb (hub->urb, GFP_ATOMIC)) != 0
> - && status != -ENODEV && status != -EPERM)
> + status = usb_submit_urb (hub->urb, GFP_ATOMIC);

checkpatch.pl should also complain about space before (.

[...]

WBR, Sergei

2015-02-09 03:26:48

by Peter Chen

[permalink] [raw]
Subject: Re: [PATCH 1/6] drivers: usb: core: devio.c: remove assignment of variables in if conditions.

On Sat, Feb 07, 2015 at 10:55:01PM +0100, Bas Peters wrote:
> This patch removes assignment of variables in if conditions in
> accordance witht the CodingStyle.

%s/witht/with

>
> Signed-off-by: Bas Peters <[email protected]>
> ---
> drivers/usb/core/devio.c | 15 ++++++++++-----
> 1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
> index 0b59731..ea3c737 100644
> --- a/drivers/usb/core/devio.c
> +++ b/drivers/usb/core/devio.c
> @@ -1081,7 +1081,8 @@ static int proc_bulk(struct usb_dev_state *ps, void __user *arg)
> ret = usbfs_increase_memory_usage(len1 + sizeof(struct urb));
> if (ret)
> return ret;
> - if (!(tbuf = kmalloc(len1, GFP_KERNEL))) {
> + tbuf = kmalloc(len1, GFP_KERNEL);
> + if (!tbuf) {
> ret = -ENOMEM;
> goto done;
> }
> @@ -1223,7 +1224,8 @@ static int proc_setintf(struct usb_dev_state *ps, void __user *arg)
>
> if (copy_from_user(&setintf, arg, sizeof(setintf)))
> return -EFAULT;
> - if ((ret = checkintf(ps, setintf.interface)))
> + ret = checkintf(ps, setintf.interface);
> + if (ret)
> return ret;
>
> destroy_async_on_interface(ps, setintf.interface);
> @@ -1392,7 +1394,8 @@ static int proc_do_submiturb(struct usb_dev_state *ps, struct usbdevfs_urb *uurb
> number_of_packets = uurb->number_of_packets;
> isofrmlen = sizeof(struct usbdevfs_iso_packet_desc) *
> number_of_packets;
> - if (!(isopkt = kmalloc(isofrmlen, GFP_KERNEL)))
> + isopkt = kmalloc(isofrmlen, GFP_KERNEL);
> + if (!isopkt)
> return -ENOMEM;
> if (copy_from_user(isopkt, iso_frame_desc, isofrmlen)) {
> ret = -EFAULT;
> @@ -1901,7 +1904,8 @@ static int proc_releaseinterface(struct usb_dev_state *ps, void __user *arg)
>
> if (get_user(ifnum, (unsigned int __user *)arg))
> return -EFAULT;
> - if ((ret = releaseintf(ps, ifnum)) < 0)
> + ret = releaseintf(ps, ifnum);
> + if (ret < 0)
> return ret;
> destroy_async_on_interface (ps, ifnum);
> return 0;
> @@ -1916,7 +1920,8 @@ static int proc_ioctl(struct usb_dev_state *ps, struct usbdevfs_ioctl *ctl)
> struct usb_driver *driver = NULL;
>
> /* alloc buffer */
> - if ((size = _IOC_SIZE(ctl->ioctl_code)) > 0) {
> + size = _IOC_SIZE(ctl->ioctl_code);
> + if (size > 0) {
> buf = kmalloc(size, GFP_KERNEL);
> if (buf == NULL)
> return -ENOMEM;
> --
> 2.1.0
>

--

Best Regards,
Peter Chen

2015-02-09 10:05:26

by Bas Peters

[permalink] [raw]
Subject: Re: [PATCH 1/6] drivers: usb: core: devio.c: remove assignment of variables in if conditions.

2015-02-09 3:15 GMT+01:00 Peter Chen <[email protected]>:
> On Sat, Feb 07, 2015 at 10:55:01PM +0100, Bas Peters wrote:
>> This patch removes assignment of variables in if conditions in
>> accordance witht the CodingStyle.
>
> %s/witht/with

Typo, my bad. Should I fix the commit message and resend?

>
>>
>> Signed-off-by: Bas Peters <[email protected]>
>> ---
>> drivers/usb/core/devio.c | 15 ++++++++++-----
>> 1 file changed, 10 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
>> index 0b59731..ea3c737 100644
>> --- a/drivers/usb/core/devio.c
>> +++ b/drivers/usb/core/devio.c
>> @@ -1081,7 +1081,8 @@ static int proc_bulk(struct usb_dev_state *ps, void __user *arg)
>> ret = usbfs_increase_memory_usage(len1 + sizeof(struct urb));
>> if (ret)
>> return ret;
>> - if (!(tbuf = kmalloc(len1, GFP_KERNEL))) {
>> + tbuf = kmalloc(len1, GFP_KERNEL);
>> + if (!tbuf) {
>> ret = -ENOMEM;
>> goto done;
>> }
>> @@ -1223,7 +1224,8 @@ static int proc_setintf(struct usb_dev_state *ps, void __user *arg)
>>
>> if (copy_from_user(&setintf, arg, sizeof(setintf)))
>> return -EFAULT;
>> - if ((ret = checkintf(ps, setintf.interface)))
>> + ret = checkintf(ps, setintf.interface);
>> + if (ret)
>> return ret;
>>
>> destroy_async_on_interface(ps, setintf.interface);
>> @@ -1392,7 +1394,8 @@ static int proc_do_submiturb(struct usb_dev_state *ps, struct usbdevfs_urb *uurb
>> number_of_packets = uurb->number_of_packets;
>> isofrmlen = sizeof(struct usbdevfs_iso_packet_desc) *
>> number_of_packets;
>> - if (!(isopkt = kmalloc(isofrmlen, GFP_KERNEL)))
>> + isopkt = kmalloc(isofrmlen, GFP_KERNEL);
>> + if (!isopkt)
>> return -ENOMEM;
>> if (copy_from_user(isopkt, iso_frame_desc, isofrmlen)) {
>> ret = -EFAULT;
>> @@ -1901,7 +1904,8 @@ static int proc_releaseinterface(struct usb_dev_state *ps, void __user *arg)
>>
>> if (get_user(ifnum, (unsigned int __user *)arg))
>> return -EFAULT;
>> - if ((ret = releaseintf(ps, ifnum)) < 0)
>> + ret = releaseintf(ps, ifnum);
>> + if (ret < 0)
>> return ret;
>> destroy_async_on_interface (ps, ifnum);
>> return 0;
>> @@ -1916,7 +1920,8 @@ static int proc_ioctl(struct usb_dev_state *ps, struct usbdevfs_ioctl *ctl)
>> struct usb_driver *driver = NULL;
>>
>> /* alloc buffer */
>> - if ((size = _IOC_SIZE(ctl->ioctl_code)) > 0) {
>> + size = _IOC_SIZE(ctl->ioctl_code);
>> + if (size > 0) {
>> buf = kmalloc(size, GFP_KERNEL);
>> if (buf == NULL)
>> return -ENOMEM;
>> --
>> 2.1.0
>>
>
> --
>
> Best Regards,
> Peter Chen

2015-02-09 10:10:20

by Bas Peters

[permalink] [raw]
Subject: Re: [PATCH 3/6] drivers: usb: core: hcd.c: remove assignment of variables in if conditions.

2015-02-08 12:15 GMT+01:00 Sergei Shtylyov <[email protected]>:
> Hello.
>
> On 2/8/2015 12:55 AM, Bas Peters wrote:
>
>> This patch removes assignment of variables in if conditions,
>> as specified in CodingStyle.
>
>
>> Signed-off-by: Bas Peters <[email protected]>
>> ---
>> drivers/usb/core/hcd.c | 15 ++++++++++-----
>> 1 file changed, 10 insertions(+), 5 deletions(-)
>
>
>> diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
>> index 11cee55..37c40d1 100644
>> --- a/drivers/usb/core/hcd.c
>> +++ b/drivers/usb/core/hcd.c
>
> [...]
>>
>> @@ -2733,7 +2736,8 @@ int usb_add_hcd(struct usb_hcd *hcd,
>> /* "reset" is misnamed; its role is now one-time init. the
>> controller
>> * should already have been reset (and boot firmware kicked off
>> etc).
>> */
>> - if (hcd->driver->reset && (retval = hcd->driver->reset(hcd)) < 0)
>> {
>> + retval = hcd->driver->reset(hcd);
>
>
> This will crash if 'hcd->driver->reset' is NULL (which is only checked
> below).
>
>> + if (hcd->driver->reset && retval < 0) {
>
>
> It wasn't equivalent change anyway as the right part of && is only
> executed if the left part is true.

I'll fix this and your other comments and resend.

>
> WBR, Sergei
>

With kind regards,

Bas