[PATCH][TRIVIAL] Bugzilla bug # 322 - double logical operator drivers/char/sx.c

[PATCH][TRIVIAL] Bugzilla bug # 322 - double logical operator drivers/char/sx.c

[Josef 'Jeff' Sipek]

Just a simple fix to make the statements more readable. (instead of
"i < TIMEOUT > 0" the statement is divided into two, joined by &&.)

Josef 'Jeff' Sipek

--- linux-2.5/drivers/char/sx.c.orig	2003-08-06 18:23:32.000000000 -0400
+++ linux-2.5/drivers/char/sx.c	2003-08-06 18:20:03.000000000 -0400
@@ -511,13 +511,13 @@
 
 	func_enter ();
 
-	for (i=0; i < TIMEOUT_1 > 0;i++) 
+	for (i=0; (i < TIMEOUT_1) && (TIMEOUT_1 > 0);i++) 
 		if ((read_sx_byte (board, offset) & mask) == correctval) {
 			func_exit ();
 			return 1;
 		}
 
-	for (i=0; i < TIMEOUT_2 > 0;i++) {
+	for (i=0; (i < TIMEOUT_2) && (TIMEOUT_2 > 0);i++) {
 		if ((read_sx_byte (board, offset) & mask) == correctval) {
 			func_exit ();
 			return 1;
@@ -537,13 +537,13 @@
 
 	func_enter ();
 
-	for (i=0; i < TIMEOUT_1 > 0;i++) 
+	for (i=0; (i < TIMEOUT_1) && (TIMEOUT_1 > 0);i++) 
 		if ((read_sx_byte (board, offset) & mask) != badval) {
 			func_exit ();
 			return 1;
 		}
 
-	for (i=0; i < TIMEOUT_2 > 0;i++) {
+	for (i=0; (i < TIMEOUT_2) && (TIMEOUT_2 > 0);i++) {
 		if ((read_sx_byte (board, offset) & mask) != badval) {
 			func_exit ();
 			return 1;

Re: [PATCH][TRIVIAL] Bugzilla bug # 322 - double logical operator drivers/char/sx.c

[Krzysztof Halasa]

Josef 'Jeff' Sipek <jeffpc@optonline.net> writes:

> Just a simple fix to make the statements more readable. (instead of
> "i < TIMEOUT > 0" the statement is divided into two, joined by &&.)
> 
> Josef 'Jeff' Sipek
> 
> --- linux-2.5/drivers/char/sx.c.orig	2003-08-06 18:23:32.000000000 -0400
> +++ linux-2.5/drivers/char/sx.c	2003-08-06 18:20:03.000000000 -0400
> @@ -511,13 +511,13 @@
>  
>  	func_enter ();
>  
> -	for (i=0; i < TIMEOUT_1 > 0;i++) 
> +	for (i=0; (i < TIMEOUT_1) && (TIMEOUT_1 > 0);i++) 
>  		if ((read_sx_byte (board, offset) & mask) == correctval) {
>  			func_exit ();
>  			return 1;
>  		}
>  

While the first version seems to be a notation error (i < X > 0 is
equivalent to i < X) the changed version doesn't need X > 0 either
as TIMEOUT_1 (and TIMEOUT_2 respectively) is a positive #define.

So I think it should be just (i=0; i < TIMEOUT_1 ;i++) and so on.
-- 
Krzysztof Halasa
Network Administrator

Re: [PATCH][TRIVIAL] Bugzilla bug # 322 - double logical operator drivers/char/sx.c

[Timothy Miller]

Josef 'Jeff' Sipek wrote:
> Just a simple fix to make the statements more readable. (instead of
> "i < TIMEOUT > 0" the statement is divided into two, joined by &&.)
> 


Can you really DO (x < y > z) and have it work as an anded pair of 
comparisons?  Maybe this is an addition to C that I am not aware of.

I would expect (x < y > z) to be equivalent to ((x < y) > z).

Re: [PATCH][TRIVIAL] Bugzilla bug # 322 - double logical operator drivers/char/sx.c

[Jeff Sipek]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On Wednesday 06 August 2003 20:35, Timothy Miller wrote:
> Josef 'Jeff' Sipek wrote:
> > Just a simple fix to make the statements more readable. (instead of
> > "i < TIMEOUT > 0" the statement is divided into two, joined by &&.)
>
> Can you really DO (x < y > z) and have it work as an anded pair of
> comparisons?  Maybe this is an addition to C that I am not aware of.
>
> I would expect (x < y > z) to be equivalent to ((x < y) > z).

Ah, very true. I wonder what the author intended. Also, since the 'z' is 0 in 
all the cases, the statement "(i < TIMEOUT) > 0" can be reduced to "i < 
TIMEOUT".

Jeff.

- -- 
Trust me, you don't want me doing _anything_ first thing in the morning.
		- Linus Torvalds
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.2 (GNU/Linux)

iD8DBQE/MarKwFP0+seVj/4RAi/xAKCvFPeDmsbG88rt08wm30+l3NP90ACgv7hF
wFn/OwQnZfrJsNQOK9AAPz0=
=8DdO
-----END PGP SIGNATURE-----

Re: [PATCH][TRIVIAL] Bugzilla bug # 322 - double logical operator drivers/char/sx.c

[Josef 'Jeff' Sipek]

Here is the updated patch:

--- linux-2.5/drivers/char/sx.c.orig	2003-08-06 18:23:32.000000000 -0400
+++ linux-2.5/drivers/char/sx.c	2003-08-06 18:20:03.000000000 -0400
@@ -511,13 +511,13 @@
 
 	func_enter ();
 
-	for (i=0; i < TIMEOUT_1 > 0;i++) 
+	for (i=0; i < TIMEOUT_1 ;i++) 
 		if ((read_sx_byte (board, offset) & mask) == correctval) {
 			func_exit ();
 			return 1;
 		}
 
-	for (i=0; i < TIMEOUT_2 > 0;i++) {
+	for (i=0; i < TIMEOUT_2 ;i++) {
 		if ((read_sx_byte (board, offset) & mask) == correctval) {
 			func_exit ();
 			return 1;
@@ -537,13 +537,13 @@
 
 	func_enter ();
 
-	for (i=0; i < TIMEOUT_1 > 0;i++) 
+	for (i=0; i < TIMEOUT_1 ;i++) 
 		if ((read_sx_byte (board, offset) & mask) != badval) {
 			func_exit ();
 			return 1;
 		}
 
-	for (i=0; i < TIMEOUT_2 > 0;i++) {
+	for (i=0; i < TIMEOUT_2 ;i++) {
 		if ((read_sx_byte (board, offset) & mask) != badval) {
 			func_exit ();
 			return 1;

Re: [PATCH][TRIVIAL] Bugzilla bug # 322 - double logical operator drivers/char/sx.c

[Valdis.Kletnieks]

[-- Attachment #1: Type: text/plain, Size: 546 bytes --]

On Wed, 06 Aug 2003 21:26:30 EDT, Jeff Sipek said:

> > Can you really DO (x < y > z) and have it work as an anded pair of
> > comparisons?  Maybe this is an addition to C that I am not aware of.
> >
> > I would expect (x < y > z) to be equivalent to ((x < y) > z).
> 
> Ah, very true. I wonder what the author intended. Also, since the 'z' is 0 in
> all the cases, the statement "(i < TIMEOUT) > 0" can be reduced to "i < 
> TIMEOUT".

Of course, if the author intended (x<y) && (x > 0), you can't reduce it if
x is at all possibly negative....

[-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --]

Re: [PATCH][TRIVIAL] Bugzilla bug # 322 - double logical operator drivers/char/sx.c

[Rahul Karnik]

Valdis.Kletnieks@vt.edu wrote:
> On Wed, 06 Aug 2003 21:26:30 EDT, Jeff Sipek said:
> 
> 
>>>Can you really DO (x < y > z) and have it work as an anded pair of
>>>comparisons?  Maybe this is an addition to C that I am not aware of.
>>>
>>>I would expect (x < y > z) to be equivalent to ((x < y) > z).
>>
>>Ah, very true. I wonder what the author intended. Also, since the 'z' is 0 in
>>all the cases, the statement "(i < TIMEOUT) > 0" can be reduced to "i < 
>>TIMEOUT".
> 
> 
> Of course, if the author intended (x<y) && (x > 0), you can't reduce it if
> x is at all possibly negative....

Doesn't matter; x is a loop index incrementing from 0 in this case.

Actually (correct me if I am wrong, but doesn't:

for(int i = 0; i < TIMEOUT > 0; i++)

translate to:

for(int i = 1; i < TIMEOUT; i++)

rather than:

for(int i = 0; i < TIMEOUT; i++)?

I hav not looked at the actual context of the code, but at least 
mathematically that makes more sense to me. i should never be 0 in the 
body of the loop, methinks?

Thanks,
Rahul
-- 
Rahul Karnik
rahul@genebrew.com

Re: [PATCH][TRIVIAL] Bugzilla bug # 322 - double logical operator drivers/char/sx.c

[Rahul Karnik]

Rahul Karnik wrote:

> Actually (correct me if I am wrong, but doesn't:
> 
> for(int i = 0; i < TIMEOUT > 0; i++)
> 
> translate to:
> 
> for(int i = 1; i < TIMEOUT; i++)
> 
> rather than:
> 
> for(int i = 0; i < TIMEOUT; i++)?

Never mind. I was smoking something.

Thanks,
Rahul
-- 
Rahul Karnik
rahul@genebrew.com

Re: [PATCH][TRIVIAL] Bugzilla bug # 322 - double logical operator drivers/char/sx.c

[Jeff Sipek]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On Wednesday 06 August 2003 20:35, Timothy Miller wrote:
> Josef 'Jeff' Sipek wrote:
> > Just a simple fix to make the statements more readable. (instead of
> > "i < TIMEOUT > 0" the statement is divided into two, joined by &&.)
>
> Can you really DO (x < y > z) and have it work as an anded pair of
> comparisons?  Maybe this is an addition to C that I am not aware of.
>
> I would expect (x < y > z) to be equivalent to ((x < y) > z).

Odd, this has been in the kernel ever since Linus started using BK. I didn't 
check pre-BK. I wonder what the author intended to say. (I believe in the 
((a<b) && (b>c)) theory.)

Jeff.

- -- 
Don't drink and derive. Alcohol and algebra don't mix.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.2 (GNU/Linux)

iD8DBQE/MgvZwFP0+seVj/4RAjOUAKCVhi17CzCtv+4jhoKg04BkDBftiwCfeZUW
ri0/IEXltFT73QrOfFsRwcU=
=vd/t
-----END PGP SIGNATURE-----

Re: [PATCH][TRIVIAL] Bugzilla bug # 322 - double logical operator drivers/char/sx.c

[jw schultz]

On Thu, Aug 07, 2003 at 04:20:37AM -0400, Jeff Sipek wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> On Wednesday 06 August 2003 20:35, Timothy Miller wrote:
> > Josef 'Jeff' Sipek wrote:
> > > Just a simple fix to make the statements more readable. (instead of
> > > "i < TIMEOUT > 0" the statement is divided into two, joined by &&.)
> >
> > Can you really DO (x < y > z) and have it work as an anded pair of
> > comparisons?  Maybe this is an addition to C that I am not aware of.
> >
> > I would expect (x < y > z) to be equivalent to ((x < y) > z).
> 
> Odd, this has been in the kernel ever since Linus started using BK. I didn't 
> check pre-BK. I wonder what the author intended to say. (I believe in the 
> ((a<b) && (b>c)) theory.)

I've got an old system with 2.2.10 and took a look.  It
appears as though the form of the loop in may of 1999 was

	for(delay = SX_CCR_TIMEOUT; delay; delay--)

so my guess is that the changes were made around the
constant to minimise typing and progressed something like so:

	for(delay = SX_CCR_TIMEOUT; delay; delay--)
	for(delay = SX_CCR_TIMEOUT; delay > 0; delay--)
	for(delay = 0; delay < SX_CCR_TIMEOUT > 0; delay++)

with name changes somewhere in the mix.  So there was never
any intent to have a double test.  Besides comparing a
constant with another constant wouldn't make much sense.


-- 
________________________________________________________________
	J.W. Schultz            Pegasystems Technologies
	email address:		jw@pegasys.ws

		Remember Cernan and Schmitt

[PATCH][TRIVIAL] Bugzilla bug # 322 - double logical operator drivers/char/sx.c

[Josef 'Jeff' Sipek]

Simple clean up patch to remove double logical operators.

Josef 'Jeff' Sipek

--- linux-2.5/drivers/char/sx.c.orig	2003-08-06 18:23:32.000000000 -0400
+++ linux-2.5/drivers/char/sx.c	2003-08-06 18:20:03.000000000 -0400
@@ -511,13 +511,13 @@
 
 	func_enter ();
 
-	for (i=0; i < TIMEOUT_1 > 0;i++) 
+	for (i=0; i < TIMEOUT_1 ;i++) 
 		if ((read_sx_byte (board, offset) & mask) == correctval) {
 			func_exit ();
 			return 1;
 		}
 
-	for (i=0; i < TIMEOUT_2 > 0;i++) {
+	for (i=0; i < TIMEOUT_2 ;i++) {
 		if ((read_sx_byte (board, offset) & mask) == correctval) {
 			func_exit ();
 			return 1;
@@ -537,13 +537,13 @@
 
 	func_enter ();
 
-	for (i=0; i < TIMEOUT_1 > 0;i++) 
+	for (i=0; i < TIMEOUT_1 ;i++) 
 		if ((read_sx_byte (board, offset) & mask) != badval) {
 			func_exit ();
 			return 1;
 		}
 
-	for (i=0; i < TIMEOUT_2 > 0;i++) {
+	for (i=0; i < TIMEOUT_2 ;i++) {
 		if ((read_sx_byte (board, offset) & mask) != badval) {
 			func_exit ();
 			return 1;