2012-11-16 20:19:32

by YAMANE Toshiaki

[permalink] [raw]
Subject: [PATCH] staging/serqt_usb2: Refactor qt_status_change_check() in serqt_usb2.c

Modify qt_status_change_check() and delete qt_status_change().

Signed-off-by: YAMANE Toshiaki <[email protected]>
---
drivers/staging/serqt_usb2/serqt_usb2.c | 53 +++++++++++++------------------
1 file changed, 22 insertions(+), 31 deletions(-)

diff --git a/drivers/staging/serqt_usb2/serqt_usb2.c b/drivers/staging/serqt_usb2/serqt_usb2.c
index f68a855..1b3e995 100644
--- a/drivers/staging/serqt_usb2/serqt_usb2.c
+++ b/drivers/staging/serqt_usb2/serqt_usb2.c
@@ -291,33 +291,6 @@ static void qt_interrupt_callback(struct urb *urb)
/* FIXME */
}

-static int qt_status_change(unsigned int limit,
- unsigned char *data,
- int i,
- struct quatech_port *qt_port,
- struct usb_serial_port *port)
-{
- void (*fn)(struct quatech_port *, unsigned char);
-
- if (0x00 == data[i + 2]) {
- dev_dbg(&port->dev, "Line status status.\n");
- fn = ProcessLineStatus;
- } else {
- dev_dbg(&port->dev, "Modem status status.\n");
- fn = ProcessModemStatus;
- }
-
- if (i > limit) {
- dev_dbg(&port->dev,
- "Illegal escape seuences in received data\n");
- return 0;
- }
-
- (*fn)(qt_port, data[i + 3]);
-
- return 1;
-}
-
static void qt_status_change_check(struct tty_struct *tty,
struct urb *urb,
struct quatech_port *qt_port,
@@ -334,11 +307,29 @@ static void qt_status_change_check(struct tty_struct *tty,
flag = 0;
switch (data[i + 2]) {
case 0x00:
+ if (i > (RxCount - 4)) {
+ dev_dbg(&port->dev,
+ "Illegal escape seuences in received data\n");
+ break;
+ }
+
+ ProcessLineStatus(qt_port, data[i + 3]);
+
+ i += 3;
+ flag = 1;
+ break;
+
case 0x01:
- flag = qt_status_change((RxCount - 4), data, i,
- qt_port, port);
- if (flag == 1)
- i += 3;
+ if (i > (RxCount - 4)) {
+ dev_dbg(&port->dev,
+ "Illegal escape seuences in received data\n");
+ break;
+ }
+
+ ProcessModemStatus(qt_port, data[i + 3]);
+
+ i += 3;
+ flag = 1;
break;

case 0xff:
--
1.7.9.5


2012-11-16 20:59:04

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH] staging/serqt_usb2: Refactor qt_status_change_check() in serqt_usb2.c

On Sat, Nov 17, 2012 at 05:19:22AM +0900, YAMANE Toshiaki wrote:
> Modify qt_status_change_check() and delete qt_status_change().
>

Thanks for doing that. I think it's a lot easier to read that way.

Acked-by: Dan Carpenter <[email protected]>

regards,
dan carpenter

2012-11-17 01:32:23

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH] staging/serqt_usb2: Refactor qt_status_change_check() in serqt_usb2.c

On Sat, 2012-11-17 at 05:19 +0900, YAMANE Toshiaki wrote:
> Modify qt_status_change_check() and delete qt_status_change().
>
> Signed-off-by: YAMANE Toshiaki <[email protected]>
> ---
> drivers/staging/serqt_usb2/serqt_usb2.c | 53 +++++++++++++------------------
> 1 file changed, 22 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/staging/serqt_usb2/serqt_usb2.c b/drivers/staging/serqt_usb2/serqt_usb2.c
[]
> @@ -334,11 +307,29 @@ static void qt_status_change_check(struct tty_struct *tty,
> flag = 0;
> switch (data[i + 2]) {
> case 0x00:
> + if (i > (RxCount - 4)) {
> + dev_dbg(&port->dev,
> + "Illegal escape seuences in received data\n");

trivia: seuences/sequence

> + break;
> + }
> +
> + ProcessLineStatus(qt_port, data[i + 3]);
> +
> + i += 3;

you could move the i += 3 before the ProcessLineStatus
and use data[i]

> + flag = 1;
> + break;
> +
> case 0x01:
> - flag = qt_status_change((RxCount - 4), data, i,
> - qt_port, port);
> - if (flag == 1)
> - i += 3;
> + if (i > (RxCount - 4)) {
> + dev_dbg(&port->dev,
> + "Illegal escape seuences in received data\n");

typo here too

> + break;
> + }
> +
> + ProcessModemStatus(qt_port, data[i + 3]);
> +
> + i += 3;

same i += 3

> + flag = 1;
> break;
>
> case 0xff:

What about something like:

case 0x0:
case 0x1:
if (i > (RxCount - 4)) {
dev_dbg(&port->dev,
"Illegal escape sequence in received data\n");
break;
}

if (data[i + 2] == 0x0)
ProcessLineStatus(qt_port, data[i + 3]);
else
ProcessModemStatus(qt_port, data[i + 3]);

i += 3;
flag = 1;
break;


2012-11-17 06:48:47

by YAMANE Toshiaki

[permalink] [raw]
Subject: [PATCH v2] staging/serqt_usb2: Refactor qt_status_change_check() in serqt_usb2.c

Modify qt_status_change_check() and delete qt_status_change().

Signed-off-by: YAMANE Toshiaki <[email protected]>
---
drivers/staging/serqt_usb2/serqt_usb2.c | 53 +++++++++++++------------------
1 file changed, 22 insertions(+), 31 deletions(-)

diff --git a/drivers/staging/serqt_usb2/serqt_usb2.c b/drivers/staging/serqt_usb2/serqt_usb2.c
index f68a855..13722b2 100644
--- a/drivers/staging/serqt_usb2/serqt_usb2.c
+++ b/drivers/staging/serqt_usb2/serqt_usb2.c
@@ -291,33 +291,6 @@ static void qt_interrupt_callback(struct urb *urb)
/* FIXME */
}

-static int qt_status_change(unsigned int limit,
- unsigned char *data,
- int i,
- struct quatech_port *qt_port,
- struct usb_serial_port *port)
-{
- void (*fn)(struct quatech_port *, unsigned char);
-
- if (0x00 == data[i + 2]) {
- dev_dbg(&port->dev, "Line status status.\n");
- fn = ProcessLineStatus;
- } else {
- dev_dbg(&port->dev, "Modem status status.\n");
- fn = ProcessModemStatus;
- }
-
- if (i > limit) {
- dev_dbg(&port->dev,
- "Illegal escape seuences in received data\n");
- return 0;
- }
-
- (*fn)(qt_port, data[i + 3]);
-
- return 1;
-}
-
static void qt_status_change_check(struct tty_struct *tty,
struct urb *urb,
struct quatech_port *qt_port,
@@ -334,11 +307,29 @@ static void qt_status_change_check(struct tty_struct *tty,
flag = 0;
switch (data[i + 2]) {
case 0x00:
+ if (i > (RxCount - 4)) {
+ dev_dbg(&port->dev,
+ "Illegal escape sequences in received data\n");
+ break;
+ }
+
+ i += 3;
+ ProcessLineStatus(qt_port, data[i]);
+
+ flag = 1;
+ break;
+
case 0x01:
- flag = qt_status_change((RxCount - 4), data, i,
- qt_port, port);
- if (flag == 1)
- i += 3;
+ if (i > (RxCount - 4)) {
+ dev_dbg(&port->dev,
+ "Illegal escape sequences in received data\n");
+ break;
+ }
+
+ i += 3;
+ ProcessModemStatus(qt_port, data[i]);
+
+ flag = 1;
break;

case 0xff:
--
1.7.9.5