Re: [PATCH] embarassing typo

Re: [PATCH] embarassing typo

[Vicente Feito]

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" <rbultje@ronald.bitfreak.net> 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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] embarassing typo

[Måns Rullgård]

Vicente Feito <vicente.feito@gmail.com> writes:

> On Tuesday 29 March 2005 09:58 pm, you wrote:
>> Måns Rullgård wrote:
>> > "Ronald S. Bultje" <rbultje@ronald.bitfreak.net> 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
mru@inprovide.com

[PATCH] embarassing typo

[Ronald S. Bultje]

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

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 <rbultje@ronald.bitfreak.net>

Thanks,

Ronald

-- 
Ronald S. Bultje <rbultje@ronald.bitfreak.net>

[-- Attachment #2: x --]
[-- Type: text/plain, Size: 577 bytes --]

? .tmp_versions
Index: zr36050.c
===================================================================
RCS file: /cvsroot/mjpeg/driver-zoran/zr36050.c,v
retrieving revision 1.2
diff -u -r1.2 zr36050.c
--- 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;
 	return zr36050_pushit(ptr, ZR050_DRI_IDX, 6, dri_data);
 }
 

Re: [PATCH] embarassing typo

[Måns Rullgård]

"Ronald S. Bultje" <rbultje@ronald.bitfreak.net> 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
mru@inprovide.com

Re: [PATCH] embarassing typo

[Michael Tokarev]

Måns Rullgård wrote:
> "Ronald S. Bultje" <rbultje@ronald.bitfreak.net> 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

Re: [PATCH] embarassing typo

[Dmitry Torokhov]

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

Re: [PATCH] embarassing typo

[Gene Heskett]

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.

Re: [PATCH] embarassing typo

[John Pearson]

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: jpearson@oasissystems.com.au

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.

Re: [PATCH] embarassing typo

[Gene Heskett]

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.

Re: [PATCH] embarassing typo

[Måns Rullgård]

Michael Tokarev <mjt@tls.msk.ru> writes:

> Måns Rullgård wrote:
>> "Ronald S. Bultje" <rbultje@ronald.bitfreak.net> 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
mru@inprovide.com

Re: [PATCH] embarassing typo

[Phil Howard]

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

| Michael Tokarev <mjt@tls.msk.ru> writes:
| 
| > M?ns Rullg?rd wrote:
| >> "Ronald S. Bultje" <rbultje@ronald.bitfreak.net> 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/ |
-----------------------------------------------------------------------------