* [PATCH] staging/serqt_usb2: Refactor qt_status_change_check() in serqt_usb2.c
@ 2012-11-29 4:57 YAMANE Toshiaki
2012-11-30 2:10 ` Greg Kroah-Hartman
0 siblings, 1 reply; 8+ messages in thread
From: YAMANE Toshiaki @ 2012-11-29 4:57 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Joe Perches, Dan Carpenter, devel, linux-kernel, YAMANE Toshiaki
Improved position to increment variable i,
And typo fixes.
Signed-off-by: YAMANE Toshiaki <yamanetoshi@gmail.com>
---
drivers/staging/serqt_usb2/serqt_usb2.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/staging/serqt_usb2/serqt_usb2.c b/drivers/staging/serqt_usb2/serqt_usb2.c
index 1b3e995..095d6f2 100644
--- a/drivers/staging/serqt_usb2/serqt_usb2.c
+++ b/drivers/staging/serqt_usb2/serqt_usb2.c
@@ -309,26 +309,26 @@ static void qt_status_change_check(struct tty_struct *tty,
case 0x00:
if (i > (RxCount - 4)) {
dev_dbg(&port->dev,
- "Illegal escape seuences in received data\n");
+ "Illegal escape sequence in received data\n");
break;
}
- ProcessLineStatus(qt_port, data[i + 3]);
-
i += 3;
+ ProcessLineStatus(qt_port, data[i]);
+
flag = 1;
break;
case 0x01:
if (i > (RxCount - 4)) {
dev_dbg(&port->dev,
- "Illegal escape seuences in received data\n");
+ "Illegal escape sequence in received data\n");
break;
}
- ProcessModemStatus(qt_port, data[i + 3]);
-
i += 3;
+ ProcessModemStatus(qt_port, data[i]);
+
flag = 1;
break;
--
1.7.9.5
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] staging/serqt_usb2: Refactor qt_status_change_check() in serqt_usb2.c
2012-11-29 4:57 [PATCH] staging/serqt_usb2: Refactor qt_status_change_check() in serqt_usb2.c YAMANE Toshiaki
@ 2012-11-30 2:10 ` Greg Kroah-Hartman
2012-11-30 5:25 ` YAMANE Toshiaki
2012-11-30 8:25 ` Dan Carpenter
0 siblings, 2 replies; 8+ messages in thread
From: Greg Kroah-Hartman @ 2012-11-30 2:10 UTC (permalink / raw)
To: YAMANE Toshiaki; +Cc: Joe Perches, devel, linux-kernel, Dan Carpenter
On Thu, Nov 29, 2012 at 01:57:56PM +0900, YAMANE Toshiaki wrote:
> Improved position to increment variable i,
> And typo fixes.
>
> Signed-off-by: YAMANE Toshiaki <yamanetoshi@gmail.com>
> ---
> drivers/staging/serqt_usb2/serqt_usb2.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/staging/serqt_usb2/serqt_usb2.c b/drivers/staging/serqt_usb2/serqt_usb2.c
> index 1b3e995..095d6f2 100644
> --- a/drivers/staging/serqt_usb2/serqt_usb2.c
> +++ b/drivers/staging/serqt_usb2/serqt_usb2.c
> @@ -309,26 +309,26 @@ static void qt_status_change_check(struct tty_struct *tty,
> case 0x00:
> if (i > (RxCount - 4)) {
> dev_dbg(&port->dev,
> - "Illegal escape seuences in received data\n");
> + "Illegal escape sequence in received data\n");
This is a different type of fix from:
> break;
> }
>
> - ProcessLineStatus(qt_port, data[i + 3]);
> -
> i += 3;
> + ProcessLineStatus(qt_port, data[i]);
I think you just changed the logic in this function, didn't you?
> +
> flag = 1;
> break;
>
> case 0x01:
> if (i > (RxCount - 4)) {
> dev_dbg(&port->dev,
> - "Illegal escape seuences in received data\n");
> + "Illegal escape sequence in received data\n");
> break;
> }
>
> - ProcessModemStatus(qt_port, data[i + 3]);
> -
> i += 3;
> + ProcessModemStatus(qt_port, data[i]);
Same here, what happens to i after this?
Please break into two patches, and verify that you didn't break anything
here.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] staging/serqt_usb2: Refactor qt_status_change_check() in serqt_usb2.c
2012-11-30 2:10 ` Greg Kroah-Hartman
@ 2012-11-30 5:25 ` YAMANE Toshiaki
2012-11-30 8:25 ` Dan Carpenter
1 sibling, 0 replies; 8+ messages in thread
From: YAMANE Toshiaki @ 2012-11-30 5:25 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: Joe Perches, devel, linux-kernel, Dan Carpenter
On Fri, Nov 30, 2012 at 11:10 AM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Thu, Nov 29, 2012 at 01:57:56PM +0900, YAMANE Toshiaki wrote:
>> Improved position to increment variable i,
>> And typo fixes.
>>
>> Signed-off-by: YAMANE Toshiaki <yamanetoshi@gmail.com>
>> ---
>> drivers/staging/serqt_usb2/serqt_usb2.c | 12 ++++++------
>> 1 file changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/staging/serqt_usb2/serqt_usb2.c b/drivers/staging/serqt_usb2/serqt_usb2.c
>> index 1b3e995..095d6f2 100644
>> --- a/drivers/staging/serqt_usb2/serqt_usb2.c
>> +++ b/drivers/staging/serqt_usb2/serqt_usb2.c
>> @@ -309,26 +309,26 @@ static void qt_status_change_check(struct tty_struct *tty,
>> case 0x00:
>> if (i > (RxCount - 4)) {
>> dev_dbg(&port->dev,
>> - "Illegal escape seuences in received data\n");
>> + "Illegal escape sequence in received data\n");
>
> This is a different type of fix from:
>
>> break;
>> }
>>
>> - ProcessLineStatus(qt_port, data[i + 3]);
>> -
>> i += 3;
>> + ProcessLineStatus(qt_port, data[i]);
>
> I think you just changed the logic in this function, didn't you?
>
>> +
>> flag = 1;
>> break;
>>
>> case 0x01:
>> if (i > (RxCount - 4)) {
>> dev_dbg(&port->dev,
>> - "Illegal escape seuences in received data\n");
>> + "Illegal escape sequence in received data\n");
>> break;
>> }
>>
>> - ProcessModemStatus(qt_port, data[i + 3]);
>> -
>> i += 3;
>> + ProcessModemStatus(qt_port, data[i]);
>
> Same here, what happens to i after this?
>
> Please break into two patches, and verify that you didn't break anything
> here.
Greg-san,
I am sorry for confusion.
I sent the patch twice since following patch was applied (gregkh/staging-next)
commit 9d36976fad3008fcc4209789566f7f3e7763f212
Modify qt_status_change_check() and delete qt_status_change().
-Incorporate comment of Mr.Joe Perches (sent Nov.17)
-I sent yesterday
Please discard the patches.
Thanks,
YAMANE Toshiaki
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] staging/serqt_usb2: Refactor qt_status_change_check() in serqt_usb2.c
2012-11-30 2:10 ` Greg Kroah-Hartman
2012-11-30 5:25 ` YAMANE Toshiaki
@ 2012-11-30 8:25 ` Dan Carpenter
2012-11-30 12:53 ` YAMANE Toshiaki
1 sibling, 1 reply; 8+ messages in thread
From: Dan Carpenter @ 2012-11-30 8:25 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: YAMANE Toshiaki, Joe Perches, devel, linux-kernel
On Thu, Nov 29, 2012 at 06:10:26PM -0800, Greg Kroah-Hartman wrote:
> On Thu, Nov 29, 2012 at 01:57:56PM +0900, YAMANE Toshiaki wrote:
> >
> > - ProcessLineStatus(qt_port, data[i + 3]);
> > -
> > i += 3;
> > + ProcessLineStatus(qt_port, data[i]);
>
> I think you just changed the logic in this function, didn't you?
This should be ok. The i += 3 was there in the original, it's just
moved up a line.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] staging/serqt_usb2: Refactor qt_status_change_check() in serqt_usb2.c
2012-11-30 8:25 ` Dan Carpenter
@ 2012-11-30 12:53 ` YAMANE Toshiaki
0 siblings, 0 replies; 8+ messages in thread
From: YAMANE Toshiaki @ 2012-11-30 12:53 UTC (permalink / raw)
To: Greg Kroah-Hartman, Dan Carpenter; +Cc: Joe Perches, devel, linux-kernel
On Fri, Nov 30, 2012 at 5:25 PM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> On Thu, Nov 29, 2012 at 06:10:26PM -0800, Greg Kroah-Hartman wrote:
>> On Thu, Nov 29, 2012 at 01:57:56PM +0900, YAMANE Toshiaki wrote:
>> >
>> > - ProcessLineStatus(qt_port, data[i + 3]);
>> > -
>> > i += 3;
>> > + ProcessLineStatus(qt_port, data[i]);
>>
>> I think you just changed the logic in this function, didn't you?
>
> This should be ok. The i += 3 was there in the original, it's just
> moved up a line.
Thanks for your kindness reply.
But please discard this patch.
Let me consider this patch again.
Regards,
YAMANE Toshiaki
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] staging/serqt_usb2: Refactor qt_status_change_check() in serqt_usb2.c
2012-11-16 20:19 YAMANE Toshiaki
2012-11-16 20:58 ` Dan Carpenter
@ 2012-11-17 1:32 ` Joe Perches
1 sibling, 0 replies; 8+ messages in thread
From: Joe Perches @ 2012-11-17 1:32 UTC (permalink / raw)
To: YAMANE Toshiaki; +Cc: Greg Kroah-Hartman, Dan Carpenter, devel, linux-kernel
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 <yamanetoshi@gmail.com>
> ---
> 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;
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] staging/serqt_usb2: Refactor qt_status_change_check() in serqt_usb2.c
2012-11-16 20:19 YAMANE Toshiaki
@ 2012-11-16 20:58 ` Dan Carpenter
2012-11-17 1:32 ` Joe Perches
1 sibling, 0 replies; 8+ messages in thread
From: Dan Carpenter @ 2012-11-16 20:58 UTC (permalink / raw)
To: YAMANE Toshiaki; +Cc: Greg Kroah-Hartman, devel, linux-kernel
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 <dan.carpenter@oracle.com>
regards,
dan carpenter
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH] staging/serqt_usb2: Refactor qt_status_change_check() in serqt_usb2.c
@ 2012-11-16 20:19 YAMANE Toshiaki
2012-11-16 20:58 ` Dan Carpenter
2012-11-17 1:32 ` Joe Perches
0 siblings, 2 replies; 8+ messages in thread
From: YAMANE Toshiaki @ 2012-11-16 20:19 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: Dan Carpenter, devel, linux-kernel, YAMANE Toshiaki
Modify qt_status_change_check() and delete qt_status_change().
Signed-off-by: YAMANE Toshiaki <yamanetoshi@gmail.com>
---
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
^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2012-11-30 12:53 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-11-29 4:57 [PATCH] staging/serqt_usb2: Refactor qt_status_change_check() in serqt_usb2.c YAMANE Toshiaki
2012-11-30 2:10 ` Greg Kroah-Hartman
2012-11-30 5:25 ` YAMANE Toshiaki
2012-11-30 8:25 ` Dan Carpenter
2012-11-30 12:53 ` YAMANE Toshiaki
-- strict thread matches above, loose matches on Subject: below --
2012-11-16 20:19 YAMANE Toshiaki
2012-11-16 20:58 ` Dan Carpenter
2012-11-17 1:32 ` Joe Perches
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).