* [Suggestion] drivers/usb/serial: looping issue: return value may override by looping
@ 2013-04-01 2:52 Chen Gang
2013-04-01 3:05 ` Greg KH
0 siblings, 1 reply; 6+ messages in thread
From: Chen Gang @ 2013-04-01 2:52 UTC (permalink / raw)
To: jhovold, Jiri Slaby, mferrell; +Cc: Greg KH, linux-kernel@vger.kernel.org
Hello Maintainers:
in drivers/usb/serial/mos7840.c:
in 'for' looping (line 632..657)
the return value 'rv' may override if not have a check in time (line 654)
next checking can not find failure which generated during looping (line 658)
please help check, thanks.
gchen.
579 static void mos7840_interrupt_callback(struct urb *urb)
580 {
581 int result;
582 int length;
583 struct moschip_port *mos7840_port;
584 struct usb_serial *serial;
585 __u16 Data;
586 unsigned char *data;
587 __u8 sp[5], st;
588 int i, rv = 0;
589 __u16 wval, wreg = 0;
590 int status = urb->status;
591
592 switch (status) {
593 case 0:
594 /* success */
595 break;
596 case -ECONNRESET:
597 case -ENOENT:
598 case -ESHUTDOWN:
599 /* this urb is terminated, clean up */
600 dev_dbg(&urb->dev->dev, "%s - urb shutting down with status: %d\n",
601 __func__, status);
602 return;
603 default:
604 dev_dbg(&urb->dev->dev, "%s - nonzero urb status received: %d\n",
605 __func__, status);
606 goto exit;
607 }
608
609 length = urb->actual_length;
610 data = urb->transfer_buffer;
611
612 serial = urb->context;
613
614 /* Moschip get 5 bytes
615 * Byte 1 IIR Port 1 (port.number is 0)
616 * Byte 2 IIR Port 2 (port.number is 1)
617 * Byte 3 IIR Port 3 (port.number is 2)
618 * Byte 4 IIR Port 4 (port.number is 3)
619 * Byte 5 FIFO status for both */
620
621 if (length && length > 5) {
622 dev_dbg(&urb->dev->dev, "%s", "Wrong data !!!\n");
623 return;
624 }
625
626 sp[0] = (__u8) data[0];
627 sp[1] = (__u8) data[1];
628 sp[2] = (__u8) data[2];
629 sp[3] = (__u8) data[3];
630 st = (__u8) data[4];
631
632 for (i = 0; i < serial->num_ports; i++) {
633 mos7840_port = mos7840_get_port_private(serial->port[i]);
634 wval =
635 (((__u16) serial->port[i]->number -
636 (__u16) (serial->minor)) + 1) << 8;
637 if (mos7840_port->open) {
638 if (sp[i] & 0x01) {
639 dev_dbg(&urb->dev->dev, "SP%d No Interrupt !!!\n", i);
640 } else {
641 switch (sp[i] & 0x0f) {
642 case SERIAL_IIR_RLS:
643 dev_dbg(&urb->dev->dev, "Serial Port %d: Receiver status error or \n", i);
644 dev_dbg(&urb->dev->dev, "address bit detected in 9-bit mode\n");
645 mos7840_port->MsrLsr = 1;
646 wreg = LINE_STATUS_REGISTER;
647 break;
648 case SERIAL_IIR_MS:
649 dev_dbg(&urb->dev->dev, "Serial Port %d: Modem status change\n", i);
650 mos7840_port->MsrLsr = 0;
651 wreg = MODEM_STATUS_REGISTER;
652 break;
653 }
654 rv = mos7840_get_reg(mos7840_port, wval, wreg, &Data);
655 }
656 }
657 }
658 if (!(rv < 0))
659 /* the completion handler for the control urb will resubmit */
660 return;
661 exit:
662 result = usb_submit_urb(urb, GFP_ATOMIC);
663 if (result) {
664 dev_err(&urb->dev->dev,
665 "%s - Error %d submitting interrupt urb\n",
666 __func__, result);
667 }
668 }
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Suggestion] drivers/usb/serial: looping issue: return value may override by looping
2013-04-01 2:52 [Suggestion] drivers/usb/serial: looping issue: return value may override by looping Chen Gang
@ 2013-04-01 3:05 ` Greg KH
2013-04-01 3:21 ` Chen Gang
2013-04-01 3:50 ` [PATCH] drivers/usb/serial: looping issue: avoid the return value overriden " Chen Gang
0 siblings, 2 replies; 6+ messages in thread
From: Greg KH @ 2013-04-01 3:05 UTC (permalink / raw)
To: Chen Gang; +Cc: jhovold, Jiri Slaby, mferrell, linux-kernel@vger.kernel.org
On Mon, Apr 01, 2013 at 10:52:57AM +0800, Chen Gang wrote:
> Hello Maintainers:
>
> in drivers/usb/serial/mos7840.c:
>
> in 'for' looping (line 632..657)
> the return value 'rv' may override if not have a check in time (line 654)
>
> next checking can not find failure which generated during looping (line 658)
>
> please help check, thanks.
No. Please send real patches if you think something is wrong, after
determining something is incorrect. To waste people's time like this,
checking to see if you have misunderstood something, does not scale at
all, please be more considerate.
greg k-h
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Suggestion] drivers/usb/serial: looping issue: return value may override by looping
2013-04-01 3:05 ` Greg KH
@ 2013-04-01 3:21 ` Chen Gang
2013-04-01 3:50 ` [PATCH] drivers/usb/serial: looping issue: avoid the return value overriden " Chen Gang
1 sibling, 0 replies; 6+ messages in thread
From: Chen Gang @ 2013-04-01 3:21 UTC (permalink / raw)
To: Greg KH; +Cc: jhovold, Jiri Slaby, mferrell, linux-kernel@vger.kernel.org
On 2013年04月01日 11:05, Greg KH wrote:
> No. Please send real patches if you think something is wrong, after
> determining something is incorrect. To waste people's time like this,
> checking to see if you have misunderstood something, does not scale at
> all, please be more considerate.
ok, thanks.
next, I should try to send patches at any condition.
:-)
--
Chen Gang
Asianux Corporation
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] drivers/usb/serial: looping issue: avoid the return value overriden by looping
2013-04-01 3:05 ` Greg KH
2013-04-01 3:21 ` Chen Gang
@ 2013-04-01 3:50 ` Chen Gang
2013-04-01 19:37 ` USB: mos7840: " Greg KH
1 sibling, 1 reply; 6+ messages in thread
From: Chen Gang @ 2013-04-01 3:50 UTC (permalink / raw)
To: Greg KH; +Cc: jhovold, Jiri Slaby, mferrell, linux-kernel@vger.kernel.org
inside the 'for' looping:
the return value 'rv' may override if not have a check in time.
next checking, outside the 'for' looping:
can not find failure which generated during the 'for' looping
so need let outside know about the failure.
Signed-off-by: Chen Gang <gang.chen@asianux.com>
---
drivers/usb/serial/mos7840.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/drivers/usb/serial/mos7840.c b/drivers/usb/serial/mos7840.c
index a0d5ea5..13aae1e 100644
--- a/drivers/usb/serial/mos7840.c
+++ b/drivers/usb/serial/mos7840.c
@@ -651,7 +651,9 @@ static void mos7840_interrupt_callback(struct urb *urb)
wreg = MODEM_STATUS_REGISTER;
break;
}
- rv = mos7840_get_reg(mos7840_port, wval, wreg, &Data);
+ if (mos7840_get_reg(mos7840_port, wval,
+ wreg, &Data) < 0)
+ rv = -1;
}
}
}
--
1.7.7.6
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: USB: mos7840: looping issue: avoid the return value overriden by looping
2013-04-01 3:50 ` [PATCH] drivers/usb/serial: looping issue: avoid the return value overriden " Chen Gang
@ 2013-04-01 19:37 ` Greg KH
2013-04-02 1:03 ` Chen Gang
0 siblings, 1 reply; 6+ messages in thread
From: Greg KH @ 2013-04-01 19:37 UTC (permalink / raw)
To: Chen Gang; +Cc: jhovold, Jiri Slaby, mferrell, linux-kernel@vger.kernel.org
On Mon, Apr 01, 2013 at 11:50:55AM +0800, Chen Gang wrote:
>
> inside the 'for' looping:
> the return value 'rv' may override if not have a check in time.
>
> next checking, outside the 'for' looping:
> can not find failure which generated during the 'for' looping
>
> so need let outside know about the failure.
>
>
> Signed-off-by: Chen Gang <gang.chen@asianux.com>
> ---
> drivers/usb/serial/mos7840.c | 4 +++-
> 1 files changed, 3 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/usb/serial/mos7840.c b/drivers/usb/serial/mos7840.c
> index a0d5ea5..13aae1e 100644
> --- a/drivers/usb/serial/mos7840.c
> +++ b/drivers/usb/serial/mos7840.c
> @@ -651,7 +651,9 @@ static void mos7840_interrupt_callback(struct urb *urb)
> wreg = MODEM_STATUS_REGISTER;
> break;
> }
> - rv = mos7840_get_reg(mos7840_port, wval, wreg, &Data);
> + if (mos7840_get_reg(mos7840_port, wval,
> + wreg, &Data) < 0)
> + rv = -1;
Please use a "real" error value, or even better yet, use the error value
returned from the call here.
And please identify the driver you are modifying in the Subject: line,
like I have changed this one to.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: USB: mos7840: looping issue: avoid the return value overriden by looping
2013-04-01 19:37 ` USB: mos7840: " Greg KH
@ 2013-04-02 1:03 ` Chen Gang
0 siblings, 0 replies; 6+ messages in thread
From: Chen Gang @ 2013-04-02 1:03 UTC (permalink / raw)
To: Greg KH; +Cc: jhovold, Jiri Slaby, mferrell, linux-kernel@vger.kernel.org
thank you for spending your time for this patch, especially you are
very busy.
next time, I should send patch with more careful, so can let other
members work with more efficiency.
excuse me, the next 2-3 days, I have to do another urgent things
within my company, so I will send our patch v2 within this week (if our
patch is urgent, too, please tell me, and I should try to send it in time).
welcome any suggestions or completions. thanks.
On 2013年04月02日 03:37, Greg KH wrote:
> Please use a "real" error value, or even better yet, use the error value
> returned from the call here.
>
ok, I will do.
> And please identify the driver you are modifying in the Subject: line,
> like I have changed this one to.
ok, thanks.
and next, I should use such kind of format for any sub systems.
--
Chen Gang
Asianux Corporation
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-04-02 1:03 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-04-01 2:52 [Suggestion] drivers/usb/serial: looping issue: return value may override by looping Chen Gang
2013-04-01 3:05 ` Greg KH
2013-04-01 3:21 ` Chen Gang
2013-04-01 3:50 ` [PATCH] drivers/usb/serial: looping issue: avoid the return value overriden " Chen Gang
2013-04-01 19:37 ` USB: mos7840: " Greg KH
2013-04-02 1:03 ` Chen Gang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox