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 <[email protected]>
---
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
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 <[email protected]>
> ---
> 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
2015-12-10 7:44 GMT-03:00 Johan Hovold <[email protected]>:
> 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 <[email protected]>
>> ---
>> 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
Hi,
"Geyslan G. Bem" <[email protected]> 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 <[email protected]>
Can you split this into one patch per driver, that way it's easier to
apply by different maintainers.
--
balbi
2015-12-10 11:50 GMT-03:00 Felipe Balbi <[email protected]>:
>
> Hi,
>
> "Geyslan G. Bem" <[email protected]> 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 <[email protected]>
>
> 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
"Geyslan G. Bem" <[email protected]> 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 <[email protected]>
> ---
I guess you didn't get previous comment in time; let's split this per
driver so different maintainers can pick their parts.
--
balbi
2015-12-10 12:13 GMT-03:00 Felipe Balbi <[email protected]>:
> "Geyslan G. Bem" <[email protected]> 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 <[email protected]>
>> ---
>
> 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
From: Felipe Balbi
> Sent: 10 December 2015 15:14
> "Geyslan G. Bem" <[email protected]> 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 <[email protected]>
> > ---
>
> 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
2015-12-10 12:17 GMT-03:00 David Laight <[email protected]>:
> From: Felipe Balbi
>> Sent: 10 December 2015 15:14
>> "Geyslan G. Bem" <[email protected]> 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 <[email protected]>
>> > ---
>>
>> 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
2015-12-10 12:29 GMT-03:00 Geyslan G. Bem <[email protected]>:
> 2015-12-10 12:17 GMT-03:00 David Laight <[email protected]>:
>> From: Felipe Balbi
>>> Sent: 10 December 2015 15:14
>>> "Geyslan G. Bem" <[email protected]> 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 <[email protected]>
>>> > ---
>>>
>>> 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
Hi,
"Geyslan G. Bem" <[email protected]> writes:
> 2015-12-10 12:13 GMT-03:00 Felipe Balbi <[email protected]>:
>> "Geyslan G. Bem" <[email protected]> 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 <[email protected]>
>>> ---
>>
>> 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