[PATCH] [media] ivtv-i2c: Fix two wanrings

[PATCH] [media] ivtv-i2c: Fix two wanrings

[Mauro Carvalho Chehab]

Fix two gcc warnings:

drivers/media/video/ivtv/ivtv-i2c.c:170: warning: cast from pointer to integer of different size
drivers/media/video/ivtv/ivtv-i2c.c:171: warning: cast from pointer to integer of different size
$ gcc --version
gcc (GCC) 4.1.2 20080704 (Red Hat 4.1.2-48)

They seem bogus, but, as the original code also has problems with
LE/BE, just change its implementation to be clear.

Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>

diff --git a/drivers/media/video/ivtv/ivtv-i2c.c b/drivers/media/video/ivtv/ivtv-i2c.c
index 2bed430..e103b8f 100644
--- a/drivers/media/video/ivtv/ivtv-i2c.c
+++ b/drivers/media/video/ivtv/ivtv-i2c.c
@@ -167,8 +167,8 @@ static int get_key_adaptec(struct IR_i2c *ir, u32 *ir_key, u32 *ir_raw)
 	keybuf[2] &= 0x7f;
 	keybuf[3] |= 0x80;
 
-	*ir_key = (u32) keybuf;
-	*ir_raw = (u32) keybuf;
+	*ir_key = keybuf[3] | keybuf[2] << 8 | keybuf[1] << 16 |keybuf[0] << 24;
+	*ir_raw = *ir_key;
 
 	return 1;
 }
-- 
1.7.3.4

Re: [PATCH] [media] ivtv-i2c: Fix two wanrings

[Hans Verkuil]

On Thursday, December 30, 2010 16:00:41 Mauro Carvalho Chehab wrote:
> Fix two gcc warnings:
> 
> drivers/media/video/ivtv/ivtv-i2c.c:170: warning: cast from pointer to integer of different size
> drivers/media/video/ivtv/ivtv-i2c.c:171: warning: cast from pointer to integer of different size
> $ gcc --version
> gcc (GCC) 4.1.2 20080704 (Red Hat 4.1.2-48)
> 
> They seem bogus, but, as the original code also has problems with
> LE/BE, just change its implementation to be clear.

Definitely not bogus:

unsigned char keybuf[4];

..

*ir_key = (u32) keybuf;

Here keybuf == &keybuf[0]. So you put the address of keybuf in *ir_key. Which is
indeed of a different size in the case of a 64-bit architecture.

What you probably meant to do is:

*ir_key = *(u32 *)keybuf;

Note that the code in your patch assumes that keybuf is in big-endian order. I assume
that's what it should be?

Regards,

	Hans

> Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>
> 
> diff --git a/drivers/media/video/ivtv/ivtv-i2c.c b/drivers/media/video/ivtv/ivtv-i2c.c
> index 2bed430..e103b8f 100644
> --- a/drivers/media/video/ivtv/ivtv-i2c.c
> +++ b/drivers/media/video/ivtv/ivtv-i2c.c
> @@ -167,8 +167,8 @@ static int get_key_adaptec(struct IR_i2c *ir, u32 *ir_key, u32 *ir_raw)
>  	keybuf[2] &= 0x7f;
>  	keybuf[3] |= 0x80;
>  
> -	*ir_key = (u32) keybuf;
> -	*ir_raw = (u32) keybuf;
> +	*ir_key = keybuf[3] | keybuf[2] << 8 | keybuf[1] << 16 |keybuf[0] << 24;
> +	*ir_raw = *ir_key;
>  
>  	return 1;
>  }
> 

-- 
Hans Verkuil - video4linux developer - sponsored by Cisco

Re: [PATCH] [media] ivtv-i2c: Fix two wanrings

[Mauro Carvalho Chehab]

Em 30-12-2010 13:21, Hans Verkuil escreveu:
> On Thursday, December 30, 2010 16:00:41 Mauro Carvalho Chehab wrote:
>> Fix two gcc warnings:
>>
>> drivers/media/video/ivtv/ivtv-i2c.c:170: warning: cast from pointer to integer of different size
>> drivers/media/video/ivtv/ivtv-i2c.c:171: warning: cast from pointer to integer of different size
>> $ gcc --version
>> gcc (GCC) 4.1.2 20080704 (Red Hat 4.1.2-48)
>>
>> They seem bogus, but, as the original code also has problems with
>> LE/BE, just change its implementation to be clear.
> 
> Definitely not bogus:

Yes, it is not bogus.
> 
> unsigned char keybuf[4];
> 
> ..
> 
> *ir_key = (u32) keybuf;
> 
> Here keybuf == &keybuf[0]. So you put the address of keybuf in *ir_key. Which is
> indeed of a different size in the case of a 64-bit architecture.
> 
> What you probably meant to do is:
> 
> *ir_key = *(u32 *)keybuf;

Yes, but this would lead into BE/LE issues.
> 
> Note that the code in your patch assumes that keybuf is in big-endian order. I assume
> that's what it should be?

Based on the way lirc_i2c was printing the data, this seems to be the most coherent way.
We'll only know for sure after testing the remote with the hardware.

> 
> Regards,
> 
> 	Hans
> 
>> Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>
>>
>> diff --git a/drivers/media/video/ivtv/ivtv-i2c.c b/drivers/media/video/ivtv/ivtv-i2c.c
>> index 2bed430..e103b8f 100644
>> --- a/drivers/media/video/ivtv/ivtv-i2c.c
>> +++ b/drivers/media/video/ivtv/ivtv-i2c.c
>> @@ -167,8 +167,8 @@ static int get_key_adaptec(struct IR_i2c *ir, u32 *ir_key, u32 *ir_raw)
>>  	keybuf[2] &= 0x7f;
>>  	keybuf[3] |= 0x80;
>>  
>> -	*ir_key = (u32) keybuf;
>> -	*ir_raw = (u32) keybuf;
>> +	*ir_key = keybuf[3] | keybuf[2] << 8 | keybuf[1] << 16 |keybuf[0] << 24;
>> +	*ir_raw = *ir_key;
>>  
>>  	return 1;
>>  }
>>
>