andrzej zaborowski wrote:
On 30/11/2007, Andre Przywara <[EMAIL PROTECTED]> wrote:
> These casts are not the right way to get rid of the warnings, as are
> some of the casts in other files in qemu_put_* and qemu_get_*
> arguments. In this case the warnings are true positives and the bugs
> causing the warnings have to be addressed instead of just the
> warnings.
Are you sure of that? Most of the fixes are like this:
>> - qemu_put_be32s(f, &s->count_shift);
>> + qemu_put_be32s(f, (uint32_t *)&s->count_shift);
qemu_put_be32s is (QEMUFile *f, const uint32_t *pv), but after all the
2nd argument is only a _pointer_ to an unsigned variable, the size is
the same (thanks to the C99 explicit types).
count_shift is an int so it is the machine word size, not necesarily
32-bit AFAIK. Otherwise I guess gcc wouldn't warn. You can possibly
use:
qemu_put_be32(f, s->count_shift);
Or point qemu_put_be32_s() to a int32_t/uint32_t variable.
Oh, you are right, I missed that. Will rework the patch.
What solution do you prefer for the opaque types? I have used the simple:
>> - void *args[MAX_ARGS];
>> + intptr_t args[MAX_ARGS];
A more portable and clean solution would be this:
- void *args[MAX_ARGS];
+ union
+ {
+ void* ptr;
+ int i;
+ } args[MAX_ARGS];
If you prefer this, I can change the patch accordingly.
I'm not sure why you get a warning here and I'm unable to run a build
at the moment. A void * should be able to store some (unknown size)
integer regardless of the platform.
sizeof(void*) is 8, whereas sizeof(int) is 4 on a 64bit platform. If I
assign a 32bit value to a 64bit (pointer) variable, GCC4 says:
warning: cast to pointer from integer of different size
You are right that a pointer _should_ be able to hold an integer, but
AFAIK this is not guaranteed (aka written in the standard). To avoid
those problems intptr_t was introduced.
But I think opaque types in C are broken anyway, so the union version
makes it at least more readable, since you explicitly say integer or
pointer at the assignment and usage. But I have no problem with using
intptr_t, the union was just a suggestion.
What about fixing monitor.c#monitor_handle_command in general and avoid
the opaque types at all?
But wouldn't the union version work well even on win64?
Regards,
Andre.
--
Andre Przywara
AMD-OSRC (Dresden)
Tel: x84917
