[LinuxBIOS] [PATCH] Fix various compiler warnings and potential bugs

[Roman Kononov]

On 02/01/2007 01:54 PM, Carl-Daniel Hailfinger wrote:
> Ed Swierk wrote:
>>>>  static int set_bits(uint8_t *port, uint32_t mask, uint32_t val)
>>>> @@ -228,7 +229,7 @@ static void aza_init(struct device *dev)
>>>>       if(!res)
>>>>               return;
>>>>
>>>> -     base =(uint8_t *) res->base;
>>>> +     base =(uint8_t *) ((uint32_t) res->base);
>>>>       printk_debug("base = %08x\n", base);
>>>>
>>>>       codec_mask = codec_detect(base);
>>> Are you really sure you want to cast a uint64_t to uint32_t to uint8_t
>>> * ?
>> I'm not sure. The current implementation truncates base to 32 bits, so
>> I suppose there ought to be a check that base doesn't exceed
>> 0xffffffff. But in this case the double-cast is still necessary to
>> avoid gcc's complaint about truncating while casting to a pointer.
> 
> Agreed.

Generally, everyone wants to be able to deal with 64-bit resources.
The 32-bit linuxbios does not allow to do it easily. Potentially, it
will migrate to 64-bit on some platforms. Such double cast will
create a bug in 64-bit mode. I think you should either suppress
the warning in the command line or make a function or macro that
casts integers to pointers. Do not do (uint32_t).

----------------------------
-	uint8_t str[48];
+	char str[48];
  	uint32_t *p;

  	msr_t msr;
@@ -533,7 +534,7 @@ static void amd_set_name_string_f(device
  	#endif
  	}

-	p = str;
+	p = (uint32_t *) str;
  	for(i=0;i<6;i++) {
  		msr.lo = *p;  p++; msr.hi = *p; p++;
  		wrmsr(0xc0010030+i, msr);
----------------------------
Do not do this unless you are sure that ((char)-1)-((uint8_t)-1)==0;
Sign extension in 'msr.lo = *p' may be important.

Regards,

Roman