Re: [PATCH] tty_ioctl: Fix the baud_table check in encode_baud_rate

2007-10-23 Thread Maciej W. Rozycki
On Wed, 17 Oct 2007, Roel Kluin wrote:

>   if (oclose >= abs(obaud - baud_table[i])) {
> 
> should work as well

 Hmm, I am not sure if it is clearer or produces better code, but send a 
patch -- I will not object.

  Maciej
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] tty_ioctl: Fix the baud_table check in encode_baud_rate

2007-10-23 Thread Maciej W. Rozycki
On Wed, 17 Oct 2007, Alan Cox wrote:

> >  The tty_termios_encode_baud_rate() function as defined by tty_ioctl.c has 
> > a problem with the baud_table within.  The comparison operators are 
> > reversed and as a result this table's entries never match and BOTHER is 
> > always used.
> 
> Oops. I thought I'd fixed that one. Bit bothersome its been in -mm that
> long before anyone noticed. 

 -ENOTENOUGHUSERS?

> >  Additionally this function's prototype is not exported unlike the 
> > function itself.
> 
> That bit seems out of date and a prototype was added later in -mm

 Given the function has survived for quite a while without a prototype 
exported (how to use it otherwise? -- I guess set_sgttyb() does not count 
as hardly anybody should be calling it), I suppose it is no surprise it 
was not adequately tested. ;-)

  Maciej
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] tty_ioctl: Fix the baud_table check in encode_baud_rate

2007-10-23 Thread Maciej W. Rozycki
On Wed, 17 Oct 2007, Alan Cox wrote:

   The tty_termios_encode_baud_rate() function as defined by tty_ioctl.c has 
  a problem with the baud_table within.  The comparison operators are 
  reversed and as a result this table's entries never match and BOTHER is 
  always used.
 
 Oops. I thought I'd fixed that one. Bit bothersome its been in -mm that
 long before anyone noticed. 

 -ENOTENOUGHUSERS?

   Additionally this function's prototype is not exported unlike the 
  function itself.
 
 That bit seems out of date and a prototype was added later in -mm

 Given the function has survived for quite a while without a prototype 
exported (how to use it otherwise? -- I guess set_sgttyb() does not count 
as hardly anybody should be calling it), I suppose it is no surprise it 
was not adequately tested. ;-)

  Maciej
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] tty_ioctl: Fix the baud_table check in encode_baud_rate

2007-10-23 Thread Maciej W. Rozycki
On Wed, 17 Oct 2007, Roel Kluin wrote:

   if (oclose = abs(obaud - baud_table[i])) {
 
 should work as well

 Hmm, I am not sure if it is clearer or produces better code, but send a 
patch -- I will not object.

  Maciej
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] tty_ioctl: Fix the baud_table check in encode_baud_rate

2007-10-17 Thread Alan Cox
On Tue, 16 Oct 2007 19:12:11 +0100 (BST)
"Maciej W. Rozycki" <[EMAIL PROTECTED]> wrote:

>  The tty_termios_encode_baud_rate() function as defined by tty_ioctl.c has 
> a problem with the baud_table within.  The comparison operators are 
> reversed and as a result this table's entries never match and BOTHER is 
> always used.

Oops. I thought I'd fixed that one. Bit bothersome its been in -mm that
long before anyone noticed. 

ACK the first bit.

>  Additionally this function's prototype is not exported unlike the 
> function itself.

That bit seems out of date and a prototype was added later in -mm

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] tty_ioctl: Fix the baud_table check in encode_baud_rate

2007-10-17 Thread Alan Cox
On Tue, 16 Oct 2007 19:12:11 +0100 (BST)
Maciej W. Rozycki [EMAIL PROTECTED] wrote:

  The tty_termios_encode_baud_rate() function as defined by tty_ioctl.c has 
 a problem with the baud_table within.  The comparison operators are 
 reversed and as a result this table's entries never match and BOTHER is 
 always used.

Oops. I thought I'd fixed that one. Bit bothersome its been in -mm that
long before anyone noticed. 

ACK the first bit.

  Additionally this function's prototype is not exported unlike the 
 function itself.

That bit seems out of date and a prototype was added later in -mm

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] tty_ioctl: Fix the baud_table check in encode_baud_rate

2007-10-16 Thread Roel Kluin
Since you were sending a fix, possibly I shouldn't comment on this. If so 
please disregard my
suggestion for a trivial cleanup.

Roel

Maciej W. Rozycki wrote:
> +void tty_termios_encode_baud_rate(struct ktermios *termios,
> +   speed_t ibaud, speed_t obaud)
>  {
>   int i = 0;
>   int ifound = -1, ofound = -1;
> @@ -251,12 +252,15 @@ void tty_termios_encode_baud_rate(struct
>   termios->c_cflag &= ~CBAUD;
>  
>   do {
> - if (obaud - oclose >= baud_table[i] && obaud + oclose <= 
> baud_table[i]) {
> + if (obaud - oclose <= baud_table[i] &&
> + obaud + oclose >= baud_table[i]) {

if(a - b <= c && a + b >= c)
if(a <= c + b && a + b >= c)
if(c + b >= a && a + b >= c)
if(b >= a - c && b >= c - a)
true, if:
b >= |a - c|
so
if (oclose >= abs(obaud - baud_table[i])) {

should work as well

>   termios->c_cflag |= baud_bits[i];
>   ofound = i;
>   }
> - if (ibaud - iclose >= baud_table[i] && ibaud + iclose <= 
> baud_table[i]) {
> - /* For the case input == output don't set IBAUD bits if 
> the user didn't do so */
> + if (ibaud - iclose <= baud_table[i] &&
> + ibaud + iclose >= baud_table[i]) {

similarly,
if (iclose >= abs(ibaud - baud_table[i])) {

> + /* For the case input == output don't set IBAUD bits
> +if the user didn't do so */
>   if (ofound != i || ibinput)
>   termios->c_cflag |= (baud_bits[i] << IBSHIFT);
>   ifound = i;
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH] tty_ioctl: Fix the baud_table check in encode_baud_rate

2007-10-16 Thread Maciej W. Rozycki
 The tty_termios_encode_baud_rate() function as defined by tty_ioctl.c has 
a problem with the baud_table within.  The comparison operators are 
reversed and as a result this table's entries never match and BOTHER is 
always used.

 Additionally this function's prototype is not exported unlike the 
function itself.

 This fix addresses both issues.

Signed-off-by: Maciej W. Rozycki <[EMAIL PROTECTED]>
---
 It has been tested with checkpatch.pl and at the run time, with a serial 
driver (dz) for which I have a change to be submitted soon that will make 
it use the function.  The hardware being handled only supports some 
discrete baud rates (a choice of 15 fixed settings plus an external rate 
input), which is why it wants to adjust termios settings according to the 
rate setting actually chosen.

 Please apply,

  Maciej

patch-mips-2.6.23-rc5-20070904-termios-baud-rate-0
diff -up --recursive --new-file 
linux-mips-2.6.23-rc5-20070904.macro/drivers/char/tty_ioctl.c 
linux-mips-2.6.23-rc5-20070904/drivers/char/tty_ioctl.c
--- linux-mips-2.6.23-rc5-20070904.macro/drivers/char/tty_ioctl.c   
2007-09-04 04:55:31.0 +
+++ linux-mips-2.6.23-rc5-20070904/drivers/char/tty_ioctl.c 2007-10-13 
23:07:27.0 +
@@ -227,7 +227,8 @@ EXPORT_SYMBOL(tty_termios_input_baud_rat
  * when calling this function from the driver termios handler.
  */
 
-void tty_termios_encode_baud_rate(struct ktermios *termios, speed_t ibaud, 
speed_t obaud)
+void tty_termios_encode_baud_rate(struct ktermios *termios,
+ speed_t ibaud, speed_t obaud)
 {
int i = 0;
int ifound = -1, ofound = -1;
@@ -251,12 +252,15 @@ void tty_termios_encode_baud_rate(struct
termios->c_cflag &= ~CBAUD;
 
do {
-   if (obaud - oclose >= baud_table[i] && obaud + oclose <= 
baud_table[i]) {
+   if (obaud - oclose <= baud_table[i] &&
+   obaud + oclose >= baud_table[i]) {
termios->c_cflag |= baud_bits[i];
ofound = i;
}
-   if (ibaud - iclose >= baud_table[i] && ibaud + iclose <= 
baud_table[i]) {
-   /* For the case input == output don't set IBAUD bits if 
the user didn't do so */
+   if (ibaud - iclose <= baud_table[i] &&
+   ibaud + iclose >= baud_table[i]) {
+   /* For the case input == output don't set IBAUD bits
+  if the user didn't do so */
if (ofound != i || ibinput)
termios->c_cflag |= (baud_bits[i] << IBSHIFT);
ifound = i;
diff -up --recursive --new-file 
linux-mips-2.6.23-rc5-20070904.macro/include/linux/tty.h 
linux-mips-2.6.23-rc5-20070904/include/linux/tty.h
--- linux-mips-2.6.23-rc5-20070904.macro/include/linux/tty.h2007-09-04 
04:56:20.0 +
+++ linux-mips-2.6.23-rc5-20070904/include/linux/tty.h  2007-10-13 
22:06:06.0 +
@@ -322,6 +322,8 @@ extern void tty_flip_buffer_push(struct 
 extern speed_t tty_get_baud_rate(struct tty_struct *tty);
 extern speed_t tty_termios_baud_rate(struct ktermios *termios);
 extern speed_t tty_termios_input_baud_rate(struct ktermios *termios);
+extern void tty_termios_encode_baud_rate(struct ktermios *termios,
+speed_t ibaud, speed_t obaud);
 
 extern struct tty_ldisc *tty_ldisc_ref(struct tty_struct *);
 extern void tty_ldisc_deref(struct tty_ldisc *);
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH] tty_ioctl: Fix the baud_table check in encode_baud_rate

2007-10-16 Thread Maciej W. Rozycki
 The tty_termios_encode_baud_rate() function as defined by tty_ioctl.c has 
a problem with the baud_table within.  The comparison operators are 
reversed and as a result this table's entries never match and BOTHER is 
always used.

 Additionally this function's prototype is not exported unlike the 
function itself.

 This fix addresses both issues.

Signed-off-by: Maciej W. Rozycki [EMAIL PROTECTED]
---
 It has been tested with checkpatch.pl and at the run time, with a serial 
driver (dz) for which I have a change to be submitted soon that will make 
it use the function.  The hardware being handled only supports some 
discrete baud rates (a choice of 15 fixed settings plus an external rate 
input), which is why it wants to adjust termios settings according to the 
rate setting actually chosen.

 Please apply,

  Maciej

patch-mips-2.6.23-rc5-20070904-termios-baud-rate-0
diff -up --recursive --new-file 
linux-mips-2.6.23-rc5-20070904.macro/drivers/char/tty_ioctl.c 
linux-mips-2.6.23-rc5-20070904/drivers/char/tty_ioctl.c
--- linux-mips-2.6.23-rc5-20070904.macro/drivers/char/tty_ioctl.c   
2007-09-04 04:55:31.0 +
+++ linux-mips-2.6.23-rc5-20070904/drivers/char/tty_ioctl.c 2007-10-13 
23:07:27.0 +
@@ -227,7 +227,8 @@ EXPORT_SYMBOL(tty_termios_input_baud_rat
  * when calling this function from the driver termios handler.
  */
 
-void tty_termios_encode_baud_rate(struct ktermios *termios, speed_t ibaud, 
speed_t obaud)
+void tty_termios_encode_baud_rate(struct ktermios *termios,
+ speed_t ibaud, speed_t obaud)
 {
int i = 0;
int ifound = -1, ofound = -1;
@@ -251,12 +252,15 @@ void tty_termios_encode_baud_rate(struct
termios-c_cflag = ~CBAUD;
 
do {
-   if (obaud - oclose = baud_table[i]  obaud + oclose = 
baud_table[i]) {
+   if (obaud - oclose = baud_table[i] 
+   obaud + oclose = baud_table[i]) {
termios-c_cflag |= baud_bits[i];
ofound = i;
}
-   if (ibaud - iclose = baud_table[i]  ibaud + iclose = 
baud_table[i]) {
-   /* For the case input == output don't set IBAUD bits if 
the user didn't do so */
+   if (ibaud - iclose = baud_table[i] 
+   ibaud + iclose = baud_table[i]) {
+   /* For the case input == output don't set IBAUD bits
+  if the user didn't do so */
if (ofound != i || ibinput)
termios-c_cflag |= (baud_bits[i]  IBSHIFT);
ifound = i;
diff -up --recursive --new-file 
linux-mips-2.6.23-rc5-20070904.macro/include/linux/tty.h 
linux-mips-2.6.23-rc5-20070904/include/linux/tty.h
--- linux-mips-2.6.23-rc5-20070904.macro/include/linux/tty.h2007-09-04 
04:56:20.0 +
+++ linux-mips-2.6.23-rc5-20070904/include/linux/tty.h  2007-10-13 
22:06:06.0 +
@@ -322,6 +322,8 @@ extern void tty_flip_buffer_push(struct 
 extern speed_t tty_get_baud_rate(struct tty_struct *tty);
 extern speed_t tty_termios_baud_rate(struct ktermios *termios);
 extern speed_t tty_termios_input_baud_rate(struct ktermios *termios);
+extern void tty_termios_encode_baud_rate(struct ktermios *termios,
+speed_t ibaud, speed_t obaud);
 
 extern struct tty_ldisc *tty_ldisc_ref(struct tty_struct *);
 extern void tty_ldisc_deref(struct tty_ldisc *);
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] tty_ioctl: Fix the baud_table check in encode_baud_rate

2007-10-16 Thread Roel Kluin
Since you were sending a fix, possibly I shouldn't comment on this. If so 
please disregard my
suggestion for a trivial cleanup.

Roel

Maciej W. Rozycki wrote:
 +void tty_termios_encode_baud_rate(struct ktermios *termios,
 +   speed_t ibaud, speed_t obaud)
  {
   int i = 0;
   int ifound = -1, ofound = -1;
 @@ -251,12 +252,15 @@ void tty_termios_encode_baud_rate(struct
   termios-c_cflag = ~CBAUD;
  
   do {
 - if (obaud - oclose = baud_table[i]  obaud + oclose = 
 baud_table[i]) {
 + if (obaud - oclose = baud_table[i] 
 + obaud + oclose = baud_table[i]) {

if(a - b = c  a + b = c)
if(a = c + b  a + b = c)
if(c + b = a  a + b = c)
if(b = a - c  b = c - a)
true, if:
b = |a - c|
so
if (oclose = abs(obaud - baud_table[i])) {

should work as well

   termios-c_cflag |= baud_bits[i];
   ofound = i;
   }
 - if (ibaud - iclose = baud_table[i]  ibaud + iclose = 
 baud_table[i]) {
 - /* For the case input == output don't set IBAUD bits if 
 the user didn't do so */
 + if (ibaud - iclose = baud_table[i] 
 + ibaud + iclose = baud_table[i]) {

similarly,
if (iclose = abs(ibaud - baud_table[i])) {

 + /* For the case input == output don't set IBAUD bits
 +if the user didn't do so */
   if (ofound != i || ibinput)
   termios-c_cflag |= (baud_bits[i]  IBSHIFT);
   ifound = i;
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/