Bugzilla bug # 322 - double logical operator drivers/char/sx.c

PATCH In review Josef 'Jeff' Sipek 6 participants 10 messages Aug 6, 2003 – Aug 7, 2003 Char and misc drivers

Share & tools

Josef 'Jeff' Sipek

author

2003-08-06 22:31 UTC #0 lore raw

PATCH 1/1 Bugzilla bug # 322 - double logical operator drivers/char/sx.c

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

drivers/char/sx.c renamed from drivers/char/sx.c.orig +4 -4

@@ -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;

Timothy Miller

2003-08-07 00:23 UTC #1 lore raw

Quoted 3 lines · 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 &&.)

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).

Jeff Sipek

author

2003-08-07 01:26 UTC #2 lore raw

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

Quoted 8 lines · Timothy Miller

Quoted 2 lines · 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 &&.)
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-----

[email protected]

2003-08-07 03:13 UTC #3 lore raw

Quoted 8 lines · Jeff Sipek

Quoted 4 lines
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....

Rahul Karnik

2003-08-07 03:41 UTC #4 lore raw

Quoted 6 lines · [email protected] wrote:

Quoted 3 lines · Jeff Sipek
Quoted 3 lines
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.

Quoted 8 lines

Quoted 4 lines
Quoted 1 line
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

Signature · 2 lines

Rahul Karnik
[email protected]

Rahul Karnik

2003-08-07 03:53 UTC #5 lore raw

Quoted 11 lines · Rahul Karnik

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

Signature · 2 lines

Rahul Karnik
[email protected]

Josef 'Jeff' Sipek

author

2003-08-07 01:30 UTC #6 lore raw

In reply to Timothy Miller

Here is the updated patch:

drivers/char/sx.c renamed from drivers/char/sx.c.orig +4 -4

@@ -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;

Jeff Sipek

author

2003-08-07 08:20 UTC #7 lore raw

In reply to Timothy Miller

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

Quoted 8 lines · Timothy Miller

Quoted 2 lines · 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 &&.)
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-----

jw schultz

2003-08-07 23:02 UTC #8 lore raw

Quoted 8 lines · Jeff Sipek

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Quoted 4 lines · Timothy Miller
Quoted 2 lines · 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 &&.)

Quoted 8 lines

Quoted 4 lines
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.

Signature · 5 lines

________________________________________________________________
	J.W. Schultz            Pegasystems Technologies
	email address:		[email protected]

		Remember Cernan and Schmitt

Krzysztof Halasa

2003-08-07 15:37 UTC #9 lore raw

In reply to Josef 'Jeff' Sipek

Quoted 8 lines · 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

drivers/char/sx.c line 519

Quoted 10 lines

drivers/char/sx.c renamed from drivers/char/sx.c.orig +1 -1

@@ -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.

Signature · 2 lines

Krzysztof Halasa
Network Administrator