2003-08-06 22:31:27

by Jeff Sipek

[permalink] [raw]
Subject: [PATCH][TRIVIAL] 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

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


2003-08-07 00:23:02

by Timothy Miller

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



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


2003-08-07 01:30:01

by Jeff Sipek

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

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;



2003-08-07 01:26:45

by Jeff Sipek

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

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

2003-08-07 03:13:01

by Valdis Klētnieks

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

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


Attachments:
(No filename) (226.00 B)

2003-08-07 03:41:31

by Rahul Karnik

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

[email protected] 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
[email protected]

2003-08-07 03:53:55

by Rahul Karnik

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

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
[email protected]

2003-08-07 08:20:55

by Jeff Sipek

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

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

2003-08-07 15:37:18

by Krzysztof Halasa

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

Josef 'Jeff' Sipek <[email protected]> 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

2003-08-07 23:02:58

by jw schultz

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

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: [email protected]

Remember Cernan and Schmitt