[PATCH] usb: remove redundant conditions

[PATCH] usb: remove redundant conditions

[Geyslan G. Bem]

This patch removes redundant conditions.

 - (!A || (A && B)) is the same as (!A || B).
 - (length && length > 5) can be reduced to a single evaluation.

Caught by: cppcheck

Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
---
 drivers/usb/gadget/udc/s3c-hsudc.c |  2 +-
 drivers/usb/host/fhci-sched.c      |  2 +-
 drivers/usb/musb/musb_gadget.c     |  5 ++---
 drivers/usb/serial/io_edgeport.c   | 35 ++++++++++++++---------------------
 drivers/usb/serial/mos7840.c       |  2 +-
 5 files changed, 19 insertions(+), 27 deletions(-)

diff --git a/drivers/usb/gadget/udc/s3c-hsudc.c b/drivers/usb/gadget/udc/s3c-hsudc.c
index e9def42..82a9e2a 100644
--- a/drivers/usb/gadget/udc/s3c-hsudc.c
+++ b/drivers/usb/gadget/udc/s3c-hsudc.c
@@ -569,7 +569,7 @@ static int s3c_hsudc_handle_reqfeat(struct s3c_hsudc *hsudc,
 		hsep = &hsudc->ep[ep_num];
 		switch (le16_to_cpu(ctrl->wValue)) {
 		case USB_ENDPOINT_HALT:
-			if (set || (!set && !hsep->wedge))
+			if (set || !hsep->wedge)
 				s3c_hsudc_set_halt(&hsep->ep, set);
 			return 0;
 		}
diff --git a/drivers/usb/host/fhci-sched.c b/drivers/usb/host/fhci-sched.c
index 95ca598..23ecac5 100644
--- a/drivers/usb/host/fhci-sched.c
+++ b/drivers/usb/host/fhci-sched.c
@@ -288,7 +288,7 @@ static int scan_ed_list(struct fhci_usb *usb,
 	list_for_each_entry(ed, list, node) {
 		td = ed->td_head;
 
-		if (!td || (td && td->status == USB_TD_INPROGRESS))
+		if (!td || (td->status == USB_TD_INPROGRESS))
 			continue;
 
 		if (ed->state != FHCI_ED_OPER) {
diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c
index 67ad630..87bd578 100644
--- a/drivers/usb/musb/musb_gadget.c
+++ b/drivers/usb/musb/musb_gadget.c
@@ -353,9 +353,8 @@ static void txstate(struct musb *musb, struct musb_request *req)
 					 *	1	>0	Yes(FS bulk)
 					 */
 					if (!musb_ep->hb_mult ||
-						(musb_ep->hb_mult &&
-						 can_bulk_split(musb,
-						    musb_ep->type)))
+					    can_bulk_split(musb,
+							   musb_ep->type))
 						csr |= MUSB_TXCSR_AUTOSET;
 				}
 				csr &= ~MUSB_TXCSR_P_UNDERRUN;
diff --git a/drivers/usb/serial/io_edgeport.c b/drivers/usb/serial/io_edgeport.c
index c086697..f49327d 100644
--- a/drivers/usb/serial/io_edgeport.c
+++ b/drivers/usb/serial/io_edgeport.c
@@ -1046,9 +1046,8 @@ static void edge_close(struct usb_serial_port *port)
 
 	edge_port->closePending = true;
 
-	if ((!edge_serial->is_epic) ||
-	    ((edge_serial->is_epic) &&
-	     (edge_serial->epic_descriptor.Supports.IOSPChase))) {
+	if (!edge_serial->is_epic ||
+	    edge_serial->epic_descriptor.Supports.IOSPChase) {
 		/* flush and chase */
 		edge_port->chaseResponsePending = true;
 
@@ -1061,9 +1060,8 @@ static void edge_close(struct usb_serial_port *port)
 			edge_port->chaseResponsePending = false;
 	}
 
-	if ((!edge_serial->is_epic) ||
-	    ((edge_serial->is_epic) &&
-	     (edge_serial->epic_descriptor.Supports.IOSPClose))) {
+	if (!edge_serial->is_epic ||
+	    edge_serial->epic_descriptor.Supports.IOSPClose) {
 	       /* close the port */
 		dev_dbg(&port->dev, "%s - Sending IOSP_CMD_CLOSE_PORT\n", __func__);
 		send_iosp_ext_cmd(edge_port, IOSP_CMD_CLOSE_PORT, 0);
@@ -1612,9 +1610,8 @@ static void edge_break(struct tty_struct *tty, int break_state)
 	struct edgeport_serial *edge_serial = usb_get_serial_data(port->serial);
 	int status;
 
-	if ((!edge_serial->is_epic) ||
-	    ((edge_serial->is_epic) &&
-	     (edge_serial->epic_descriptor.Supports.IOSPChase))) {
+	if (!edge_serial->is_epic ||
+	    edge_serial->epic_descriptor.Supports.IOSPChase) {
 		/* flush and chase */
 		edge_port->chaseResponsePending = true;
 
@@ -1628,9 +1625,8 @@ static void edge_break(struct tty_struct *tty, int break_state)
 		}
 	}
 
-	if ((!edge_serial->is_epic) ||
-	    ((edge_serial->is_epic) &&
-	     (edge_serial->epic_descriptor.Supports.IOSPSetClrBreak))) {
+	if (!edge_serial->is_epic ||
+	    edge_serial->epic_descriptor.Supports.IOSPSetClrBreak) {
 		if (break_state == -1) {
 			dev_dbg(&port->dev, "%s - Sending IOSP_CMD_SET_BREAK\n", __func__);
 			status = send_iosp_ext_cmd(edge_port,
@@ -2465,9 +2461,8 @@ static void change_port_settings(struct tty_struct *tty,
 		unsigned char stop_char  = STOP_CHAR(tty);
 		unsigned char start_char = START_CHAR(tty);
 
-		if ((!edge_serial->is_epic) ||
-		    ((edge_serial->is_epic) &&
-		     (edge_serial->epic_descriptor.Supports.IOSPSetXChar))) {
+		if (!edge_serial->is_epic ||
+		    edge_serial->epic_descriptor.Supports.IOSPSetXChar) {
 			send_iosp_ext_cmd(edge_port,
 					IOSP_CMD_SET_XON_CHAR, start_char);
 			send_iosp_ext_cmd(edge_port,
@@ -2494,13 +2489,11 @@ static void change_port_settings(struct tty_struct *tty,
 	}
 
 	/* Set flow control to the configured value */
-	if ((!edge_serial->is_epic) ||
-	    ((edge_serial->is_epic) &&
-	     (edge_serial->epic_descriptor.Supports.IOSPSetRxFlow)))
+	if (!edge_serial->is_epic ||
+	    edge_serial->epic_descriptor.Supports.IOSPSetRxFlow)
 		send_iosp_ext_cmd(edge_port, IOSP_CMD_SET_RX_FLOW, rxFlow);
-	if ((!edge_serial->is_epic) ||
-	    ((edge_serial->is_epic) &&
-	     (edge_serial->epic_descriptor.Supports.IOSPSetTxFlow)))
+	if (!edge_serial->is_epic ||
+	    edge_serial->epic_descriptor.Supports.IOSPSetTxFlow)
 		send_iosp_ext_cmd(edge_port, IOSP_CMD_SET_TX_FLOW, txFlow);
 
 
diff --git a/drivers/usb/serial/mos7840.c b/drivers/usb/serial/mos7840.c
index 8ac9b55..2c69bfc 100644
--- a/drivers/usb/serial/mos7840.c
+++ b/drivers/usb/serial/mos7840.c
@@ -635,7 +635,7 @@ static void mos7840_interrupt_callback(struct urb *urb)
 	 * Byte 4 IIR Port 4 (port.number is 3)
 	 * Byte 5 FIFO status for both */
 
-	if (length && length > 5) {
+	if (length > 5) {
 		dev_dbg(&urb->dev->dev, "%s", "Wrong data !!!\n");
 		return;
 	}
-- 
2.6.3

Re: [PATCH] usb: remove redundant conditions

[Johan Hovold]

On Wed, Dec 09, 2015 at 10:52:42PM -0300, Geyslan G. Bem wrote:
> This patch removes redundant conditions.
> 
>  - (!A || (A && B)) is the same as (!A || B).
>  - (length && length > 5) can be reduced to a single evaluation.
> 
> Caught by: cppcheck
> 
> Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
> ---
>  drivers/usb/gadget/udc/s3c-hsudc.c |  2 +-
>  drivers/usb/host/fhci-sched.c      |  2 +-
>  drivers/usb/musb/musb_gadget.c     |  5 ++---
>  drivers/usb/serial/io_edgeport.c   | 35 ++++++++++++++---------------------
>  drivers/usb/serial/mos7840.c       |  2 +-
>  5 files changed, 19 insertions(+), 27 deletions(-)

Please split out (at least) the usb-serial changes into two patches.

Johan

Re: [PATCH] usb: remove redundant conditions

[Geyslan G. Bem]

2015-12-10 7:44 GMT-03:00 Johan Hovold <johan@kernel.org>:
> On Wed, Dec 09, 2015 at 10:52:42PM -0300, Geyslan G. Bem wrote:
>> This patch removes redundant conditions.
>>
>>  - (!A || (A && B)) is the same as (!A || B).
>>  - (length && length > 5) can be reduced to a single evaluation.
>>
>> Caught by: cppcheck
>>
>> Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
>> ---
>>  drivers/usb/gadget/udc/s3c-hsudc.c |  2 +-
>>  drivers/usb/host/fhci-sched.c      |  2 +-
>>  drivers/usb/musb/musb_gadget.c     |  5 ++---
>>  drivers/usb/serial/io_edgeport.c   | 35 ++++++++++++++---------------------
>>  drivers/usb/serial/mos7840.c       |  2 +-
>>  5 files changed, 19 insertions(+), 27 deletions(-)
>
> Please split out (at least) the usb-serial changes into two patches.
Ok, I'll send v2 later.

>
> Johan



-- 
Regards,

Geyslan G. Bem
hackingbits.com

Re: [PATCH] usb: remove redundant conditions

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 421 bytes --]


Hi,

"Geyslan G. Bem" <geyslan@gmail.com> writes:
> This patch removes redundant conditions.
>
>  - (!A || (A && B)) is the same as (!A || B).
>  - (length && length > 5) can be reduced to a single evaluation.
>
> Caught by: cppcheck
>
> Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>

Can you split this into one patch per driver, that way it's easier to
apply by different maintainers.

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

Re: [PATCH] usb: remove redundant conditions

[Geyslan G. Bem]

2015-12-10 11:50 GMT-03:00 Felipe Balbi <balbi@ti.com>:
>
> Hi,
>
> "Geyslan G. Bem" <geyslan@gmail.com> writes:
>> This patch removes redundant conditions.
>>
>>  - (!A || (A && B)) is the same as (!A || B).
>>  - (length && length > 5) can be reduced to a single evaluation.
>>
>> Caught by: cppcheck
>>
>> Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
>
> Can you split this into one patch per driver, that way it's easier to
> apply by different maintainers.
Balbi, sure, I can.

Tks.
>
> --
> balbi



-- 
Regards,

Geyslan G. Bem
hackingbits.com

Re: [PATCH] usb: remove redundant conditions

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 443 bytes --]

"Geyslan G. Bem" <geyslan@gmail.com> writes:

> This patch removes redundant conditions.
>
>  - (!A || (A && B)) is the same as (!A || B).
>  - (length && length > 5) can be reduced to a single evaluation.
>
> Caught by: cppcheck
>
> Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
> ---

I guess you didn't get previous comment in time; let's split this per
driver so different maintainers can pick their parts.

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

Re: [PATCH] usb: remove redundant conditions

[Geyslan G. Bem]

2015-12-10 12:13 GMT-03:00 Felipe Balbi <balbi@ti.com>:
> "Geyslan G. Bem" <geyslan@gmail.com> writes:
>
>> This patch removes redundant conditions.
>>
>>  - (!A || (A && B)) is the same as (!A || B).
>>  - (length && length > 5) can be reduced to a single evaluation.
>>
>> Caught by: cppcheck
>>
>> Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
>> ---
>
> I guess you didn't get previous comment in time; let's split this per
> driver so different maintainers can pick their parts.

Okeydokey. :)
>
> --
> balbi



-- 
Regards,

Geyslan G. Bem
hackingbits.com

RE: [PATCH] usb: remove redundant conditions

[David Laight]

From: Felipe Balbi
> Sent: 10 December 2015 15:14
> "Geyslan G. Bem" <geyslan@gmail.com> writes:
> 
> > This patch removes redundant conditions.
> >
> >  - (!A || (A && B)) is the same as (!A || B).
> >  - (length && length > 5) can be reduced to a single evaluation.
> >
> > Caught by: cppcheck
> >
> > Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
> > ---
> 
> I guess you didn't get previous comment in time; let's split this per
> driver so different maintainers can pick their parts.

I also suspect that gcc will optimise out the redundant checks as well.

	David

Re: [PATCH] usb: remove redundant conditions

[Geyslan G. Bem]

2015-12-10 12:17 GMT-03:00 David Laight <David.Laight@aculab.com>:
> From: Felipe Balbi
>> Sent: 10 December 2015 15:14
>> "Geyslan G. Bem" <geyslan@gmail.com> writes:
>>
>> > This patch removes redundant conditions.
>> >
>> >  - (!A || (A && B)) is the same as (!A || B).
>> >  - (length && length > 5) can be reduced to a single evaluation.
>> >
>> > Caught by: cppcheck
>> >
>> > Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
>> > ---
>>
>> I guess you didn't get previous comment in time; let's split this per
>> driver so different maintainers can pick their parts.
>
> I also suspect that gcc will optimise out the redundant checks as well.
Yes, David. it will.

Let's see.

void f(int f, int s)
{
        if (!f || (f && s))
                printf("branch\n");
}

Generates without optimization three comparisons:

        cmpl    $0, -4(%rbp)
        je      .L2
        cmpl    $0, -4(%rbp)
        je      .L4
        cmpl    $0, -8(%rbp)
        je      .L4

But with -O2 it generates only two:

        testl   %edi, %edi
        je      .L2
        testl   %esi, %esi
        je      .L1

Despite that, I think that the patches are welcome since they silence
checkpatch and make code clearer. Don't you think?

>
>         David
>



-- 
Regards,

Geyslan G. Bem
hackingbits.com

Re: [PATCH] usb: remove redundant conditions

[Geyslan G. Bem]

2015-12-10 12:29 GMT-03:00 Geyslan G. Bem <geyslan@gmail.com>:
> 2015-12-10 12:17 GMT-03:00 David Laight <David.Laight@aculab.com>:
>> From: Felipe Balbi
>>> Sent: 10 December 2015 15:14
>>> "Geyslan G. Bem" <geyslan@gmail.com> writes:
>>>
>>> > This patch removes redundant conditions.
>>> >
>>> >  - (!A || (A && B)) is the same as (!A || B).
>>> >  - (length && length > 5) can be reduced to a single evaluation.
>>> >
>>> > Caught by: cppcheck
>>> >
>>> > Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
>>> > ---
>>>
>>> I guess you didn't get previous comment in time; let's split this per
>>> driver so different maintainers can pick their parts.
>>
>> I also suspect that gcc will optimise out the redundant checks as well.
> Yes, David. it will.
>
> Let's see.
>
> void f(int f, int s)
> {
>         if (!f || (f && s))
>                 printf("branch\n");
> }
>
> Generates without optimization three comparisons:
>
>         cmpl    $0, -4(%rbp)
>         je      .L2
>         cmpl    $0, -4(%rbp)
>         je      .L4
>         cmpl    $0, -8(%rbp)
>         je      .L4
>
> But with -O2 it generates only two:
>
>         testl   %edi, %edi
>         je      .L2
>         testl   %esi, %esi
>         je      .L1
>
> Despite that, I think that the patches are welcome since they silence
> checkpatch and make code clearer. Don't you think?
Sorry, silence "cppcheck". I'm mistaking the tool name.
>
>>
>>         David
>>
>
>
>
> --
> Regards,
>
> Geyslan G. Bem
> hackingbits.com



-- 
Regards,

Geyslan G. Bem
hackingbits.com

Re: [PATCH] usb: remove redundant conditions

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 622 bytes --]


Hi,

"Geyslan G. Bem" <geyslan@gmail.com> writes:
> 2015-12-10 12:13 GMT-03:00 Felipe Balbi <balbi@ti.com>:
>> "Geyslan G. Bem" <geyslan@gmail.com> writes:
>>
>>> This patch removes redundant conditions.
>>>
>>>  - (!A || (A && B)) is the same as (!A || B).
>>>  - (length && length > 5) can be reduced to a single evaluation.
>>>
>>> Caught by: cppcheck
>>>
>>> Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
>>> ---
>>
>> I guess you didn't get previous comment in time; let's split this per
>> driver so different maintainers can pick their parts.
>
> Okeydokey. :)

thanks :-)

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]