All of lore.kernel.org
 help / color / mirror / Atom feed
* [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

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

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

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


  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

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


  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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.