2005-03-29 20:36:42

by Ronald S. Bultje

[permalink] [raw]
Subject: [PATCH] embarassing typo

Hi Andrew,

for some unknown reason, I suddenly found the attached typo. It doesn't
cause anything bad (at least not on my computer according to some
tests), but is still very much so embarassing, so please apply to the
kernel tree. Who knows, maybe it fixes some obscure unfixeable bug for
some people.

Signed-off-by: Ronald S. Bultje <[email protected]>

Thanks,

Ronald

--
Ronald S. Bultje <[email protected]>


Attachments:
x (577.00 B)

2005-03-29 21:04:16

by Måns Rullgård

[permalink] [raw]
Subject: Re: [PATCH] embarassing typo

"Ronald S. Bultje" <[email protected]> writes:

> --- linux-2.6.5/drivers/media/video/zr36050.c.old 16 Sep 2004 22:53:27 -0000 1.2
> +++ linux-2.6.5/drivers/media/video/zr36050.c 29 Mar 2005 20:30:23 -0000
> @@ -419,7 +419,7 @@
> dri_data[2] = 0x00;
> dri_data[3] = 0x04;
> dri_data[4] = ptr->dri >> 8;
> - dri_data[5] = ptr->dri * 0xff;
> + dri_data[5] = ptr->dri & 0xff;

Hey, that's a nice obfuscation of a simple negation.

BTW, when assigning to a char type, is the masking really necessary at
all? I can't see that it should make a difference. Am I missing
something subtle?

--
M?ns Rullg?rd
[email protected]

2005-03-29 21:59:15

by Michael Tokarev

[permalink] [raw]
Subject: Re: [PATCH] embarassing typo

M?ns Rullg?rd wrote:
> "Ronald S. Bultje" <[email protected]> writes:
>
>>--- linux-2.6.5/drivers/media/video/zr36050.c.old 16 Sep 2004 22:53:27 -0000 1.2
>>+++ linux-2.6.5/drivers/media/video/zr36050.c 29 Mar 2005 20:30:23 -0000
>>@@ -419,7 +419,7 @@
>> dri_data[2] = 0x00;
>> dri_data[3] = 0x04;
>> dri_data[4] = ptr->dri >> 8;
>>- dri_data[5] = ptr->dri * 0xff;
>>+ dri_data[5] = ptr->dri & 0xff;
>
> Hey, that's a nice obfuscation of a simple negation.

It's not a negation. This statement always assigns zero to
dri_data[5] if dri_data is char[]. Looks like gcc isn't catching
this problem.

> BTW, when assigning to a char type, is the masking really necessary at
> all? I can't see that it should make a difference. Am I missing
> something subtle?

Well, it's a matter of readability mostly. For now at least, when
char is always 8 bytes...

/mjt

2005-03-30 01:42:25

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [PATCH] embarassing typo

On Tuesday 29 March 2005 16:58, Michael Tokarev wrote:
> Well, it's a matter of readability mostly. ?For now at least, when
> char is always 8 bytes...

Wow, that's one huge char you have there ;)

--
Dmitry

2005-03-30 02:08:08

by Måns Rullgård

[permalink] [raw]
Subject: Re: [PATCH] embarassing typo

Michael Tokarev <[email protected]> writes:

> M?ns Rullg?rd wrote:
>> "Ronald S. Bultje" <[email protected]> writes:
>>
>>>--- linux-2.6.5/drivers/media/video/zr36050.c.old 16 Sep 2004 22:53:27 -0000 1.2
>>>+++ linux-2.6.5/drivers/media/video/zr36050.c 29 Mar 2005 20:30:23 -0000
>>>@@ -419,7 +419,7 @@
>>> dri_data[2] = 0x00;
>>> dri_data[3] = 0x04;
>>> dri_data[4] = ptr->dri >> 8;
>>>- dri_data[5] = ptr->dri * 0xff;
>>>+ dri_data[5] = ptr->dri & 0xff;
>> Hey, that's a nice obfuscation of a simple negation.
>
> It's not a negation. This statement always assigns zero to
> dri_data[5] if dri_data is char[].

Sure about that?

__u16 i;
char c;
i = 1; c = i * 255; /* c = 255 = -1 */
i = 2; c = i * 255; /* c = 510 & 0xff = 254 = -2 */
...

Looks like negation to me.

--
M?ns Rullg?rd
[email protected]

2005-03-30 02:33:59

by Vicente Feito

[permalink] [raw]
Subject: Re: [PATCH] embarassing typo

As long as the variable doesn't get overflowed you would have a negation, you
shouldn't do dri_data[5] = ptr->dri * 0xff; if ptr->dri it's 255, but if
ptr->dri = 1 i.e. (like is set in zr36050_setup) then you would be getting
the negation, -1. the Direct rendering support is a flag afaik, so in this
case I believe is a worthy C obfuscated negation code :)
btw, are you sure about this patch?I would contact the maintainer first,
because and'ing that doesn't make much sense...
Disclaimer, all this is: AFAIK! :)

On Tuesday 29 March 2005 09:58 pm, you wrote:
> M?ns Rullg?rd wrote:
> > "Ronald S. Bultje" <[email protected]> writes:
> >>--- linux-2.6.5/drivers/media/video/zr36050.c.old 16 Sep 2004 22:53:27
> >> -0000 1.2 +++ linux-2.6.5/drivers/media/video/zr36050.c 29 Mar 2005
> >> 20:30:23 -0000 @@ -419,7 +419,7 @@
> >> dri_data[2] = 0x00;
> >> dri_data[3] = 0x04;
> >> dri_data[4] = ptr->dri >> 8;
> >>- dri_data[5] = ptr->dri * 0xff;
> >>+ dri_data[5] = ptr->dri & 0xff;
> >
> > Hey, that's a nice obfuscation of a simple negation.
>
> It's not a negation. This statement always assigns zero to
> dri_data[5] if dri_data is char[]. Looks like gcc isn't catching
> this problem.
>
> > BTW, when assigning to a char type, is the masking really necessary at
> > all? I can't see that it should make a difference. Am I missing
> > something subtle?
>
> Well, it's a matter of readability mostly. For now at least, when
> char is always 8 bytes...
>
> /mjt
> -
> 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/

2005-03-30 03:34:47

by Phil Howard

[permalink] [raw]
Subject: Re: [PATCH] embarassing typo

On Wed, Mar 30, 2005 at 04:07:39AM +0200, M?ns Rullg?rd wrote:

| Michael Tokarev <[email protected]> writes:
|
| > M?ns Rullg?rd wrote:
| >> "Ronald S. Bultje" <[email protected]> writes:
| >>
| >>>--- linux-2.6.5/drivers/media/video/zr36050.c.old 16 Sep 2004 22:53:27 -0000 1.2
| >>>+++ linux-2.6.5/drivers/media/video/zr36050.c 29 Mar 2005 20:30:23 -0000
| >>>@@ -419,7 +419,7 @@
| >>> dri_data[2] = 0x00;
| >>> dri_data[3] = 0x04;
| >>> dri_data[4] = ptr->dri >> 8;
| >>>- dri_data[5] = ptr->dri * 0xff;
| >>>+ dri_data[5] = ptr->dri & 0xff;
| >> Hey, that's a nice obfuscation of a simple negation.
| >
| > It's not a negation. This statement always assigns zero to
| > dri_data[5] if dri_data is char[].
|
| Sure about that?
|
| __u16 i;
| char c;
| i = 1; c = i * 255; /* c = 255 = -1 */
| i = 2; c = i * 255; /* c = 510 & 0xff = 254 = -2 */
| ...
|
| Looks like negation to me.

Sure it's negation because 255 _is_ 256 - 1. Basic finite math.

( x * 256 ) mod 256 == 0
( ( x * 256 ) - ( x * 1 ) ) mod 256 == - ( x * 1 )
( x * ( 256 - 1 ) ) mod 256 == - ( x * 1 )
( x * 255 ) mod 256 == - ( x * 1 )
( x * 255 ) mod 256 == - x

Now what I am interested in is if gcc optimized it to a faster negation
or subtraction instruction.

--
-----------------------------------------------------------------------------
| Phil Howard KA9WGN | http://linuxhomepage.com/ http://ham.org/ |
| (first name) at ipal.net | http://phil.ipal.org/ http://ka9wgn.ham.org/ |
-----------------------------------------------------------------------------

2005-03-30 03:36:28

by Måns Rullgård

[permalink] [raw]
Subject: Re: [PATCH] embarassing typo

Vicente Feito <[email protected]> writes:

> On Tuesday 29 March 2005 09:58 pm, you wrote:
>> M?ns Rullg?rd wrote:
>> > "Ronald S. Bultje" <[email protected]> writes:
>> >>--- linux-2.6.5/drivers/media/video/zr36050.c.old 16 Sep 2004 22:53:27
>> >> -0000 1.2 +++ linux-2.6.5/drivers/media/video/zr36050.c 29 Mar 2005
>> >> 20:30:23 -0000 @@ -419,7 +419,7 @@
>> >> dri_data[2] = 0x00;
>> >> dri_data[3] = 0x04;
>> >> dri_data[4] = ptr->dri >> 8;
>> >>- dri_data[5] = ptr->dri * 0xff;
>> >>+ dri_data[5] = ptr->dri & 0xff;
>> >
>> > Hey, that's a nice obfuscation of a simple negation.
>>
>> It's not a negation. This statement always assigns zero to
>> dri_data[5] if dri_data is char[]. Looks like gcc isn't catching
>> this problem.
>>
> As long as the variable doesn't get overflowed you would have a
> negation, you shouldn't do dri_data[5] = ptr->dri * 0xff; if
> ptr->dri it's 255, but if ptr->dri = 1 i.e. (like is set in
> zr36050_setup) then you would be getting the negation, -1. the
> Direct rendering support is a flag afaik, so in this case I believe
> is a worthy C obfuscated negation code :)
> btw, are you sure about this patch?I would contact the maintainer
> first, because and'ing that doesn't make much sense...

It seems pretty obvious to me, that the code is supposed to store the
high byte in dri_data[4], and the low byte in dri_data[5]. Mistyping
& as * doesn't seem too unlikely, either.

--
M?ns Rullg?rd
[email protected]

2005-03-30 05:53:45

by Gene Heskett

[permalink] [raw]
Subject: Re: [PATCH] embarassing typo

On Tuesday 29 March 2005 20:40, Dmitry Torokhov wrote:
>On Tuesday 29 March 2005 16:58, Michael Tokarev wrote:
>> Well, it's a matter of readability mostly. ?For now at least, when
>> char is always 8 bytes...
>
>Wow, that's one huge char you have there ;)

Yeah, I was gonna ask what language is so complex as to need an 8 byte
char?

Certainly not an earthly one I'd think ;)

--
Cheers, Gene
"There are four boxes to be used in defense of liberty:
soap, ballot, jury, and ammo. Please use in that order."
-Ed Howdershelt (Author)
99.34% setiathome rank, not too shabby for a WV hillbilly
Yahoo.com and AOL/TW attorneys please note, additions to the above
message by Gene Heskett are:
Copyright 2005 by Maurice Eugene Heskett, all rights reserved.

2005-03-30 23:39:03

by jpearson

[permalink] [raw]
Subject: Re: [PATCH] embarassing typo

On Wed, Mar 30, 2005 at 12:53:28AM -0500, Gene Heskett wrote
> On Tuesday 29 March 2005 20:40, Dmitry Torokhov wrote:
> >On Tuesday 29 March 2005 16:58, Michael Tokarev wrote:
> >> Well, it's a matter of readability mostly. ?For now at least, when
> >> char is always 8 bytes...
> >
> >Wow, that's one huge char you have there ;)
>
> Yeah, I was gonna ask what language is so complex as to need an 8 byte
> char?
>
> Certainly not an earthly one I'd think ;)
>

Might come in handy for Perl 7 Regular Expression syntax...


John.
--
Voice: +61 8 8202 9040
Email: [email protected]

Oasis Systems Pty Ltd
288 Glen Osmond Road
Fullarton, South Australia 5063

Ph: + 61 8 82029000
Fax: +61 8 82029001

CAUTION: This email and any attachments may contain information that is
confidential and subject to copyright. If you are not the
intended recipient, you must not read, use, disseminate, distribute or
copy this email or any attachments. If you have received this
email in error, please notify the sender immediately by reply email and
erase this email and any attachments.

DISCLAIMER: OASIS Systems uses virus-scanning technology but accepts
no responsibility for loss or damage arising from the use of the
information transmitted by this email including damage from virus.

2005-03-31 04:16:50

by Gene Heskett

[permalink] [raw]
Subject: Re: [PATCH] embarassing typo

On Wednesday 30 March 2005 18:38, John Pearson wrote:
>On Wed, Mar 30, 2005 at 12:53:28AM -0500, Gene Heskett wrote
>
>> On Tuesday 29 March 2005 20:40, Dmitry Torokhov wrote:
>> >On Tuesday 29 March 2005 16:58, Michael Tokarev wrote:
>> >> Well, it's a matter of readability mostly. ?For now at least,
>> >> when char is always 8 bytes...
>> >
>> >Wow, that's one huge char you have there ;)
>>
>> Yeah, I was gonna ask what language is so complex as to need an 8
>> byte char?
>>
>> Certainly not an earthly one I'd think ;)
>
>Might come in handy for Perl 7 Regular Expression syntax...

Humm, yes, perl. I'd forgotten that. Would an 8 byte char be enough
in that case?

>John.

--
Cheers, Gene
"There are four boxes to be used in defense of liberty:
soap, ballot, jury, and ammo. Please use in that order."
-Ed Howdershelt (Author)
99.34% setiathome rank, not too shabby for a WV hillbilly
Yahoo.com and AOL/TW attorneys please note, additions to the above
message by Gene Heskett are:
Copyright 2005 by Maurice Eugene Heskett, all rights reserved.