[LinuxBIOS] [PATCH] Fix various compiler warnings and potential bugs
Roman Kononov
kononov195-lbl at yahoo.com
Thu Feb 1 21:17:09 CET 2007
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
More information about the coreboot
mailing list