Re: [Qemu-devel] [PATCH] Fix conversions from pointer to int and vice versa

[Stefan Weil <weil@mail.berlios.de>]

Am 24.02.2011 20:57, schrieb Stefan Weil:
> Am 24.02.2011 11:11, schrieb Markus Armbruster:
>> Kevin Wolf <kwolf@redhat.com> writes:
>>> Am 24.02.2011 08:21, schrieb Markus Armbruster:
>>>> Stefan Weil <weil@mail.berlios.de> writes:
>>>>> Here the int values fds[0], sigfd, s, sock and fd are converted
>>>>> to void pointers which are later converted back to an int value.
>>>>>
>>>>> These conversions should always use intptr_t instead of unsigned 
>>>>> long.
>>>>>
>>>>> They are needed for environments where sizeof(long) != sizeof(void 
>>>>> *).
>>>> To be precise: when you want to cast a pointer to a signed integer 
>>>> type
>>>> and back without loss, intptr_t is the signed integer type to use.
>>>>
>>>> But here we're dealing with the opposite case: cast int to pointer and
>>>> back.
>>>>> Signed-off-by: Stefan Weil <weil@mail.berlios.de>
>>>>> ---
>>>>> cpus.c | 8 ++++----
>>>>> migration-tcp.c | 4 ++--
>>>>> migration-unix.c | 4 ++--
>>>>> qemu-char.c | 4 ++--
>>>>> 4 files changed, 10 insertions(+), 10 deletions(-)
>>>>>
>>>>> diff --git a/cpus.c b/cpus.c
>>>>> index 0f33945..3c4e1b8 100644
>>>>> --- a/cpus.c
>>>>> +++ b/cpus.c
>>>>> @@ -267,7 +267,7 @@ static void qemu_event_increment(void)
>>>>>
>>>>> static void qemu_event_read(void *opaque)
>>>>> {
>>>>> - int fd = (unsigned long)opaque;
>>>>> + int fd = (intptr_t)opaque;
>>>>> ssize_t len;
>>>>> char buffer[512];
>>>> Why can't you cast straight to int?
>>> You would get warnings about a pointer being cast to an integer of
>>> different size
>> Fair enough. Stop reading here unless you like language-lawyering ;)
>>> (the behaviour is undefined if the integer is too small).
>>
>> Correct (I looked it up). The detour via intptr_t makes it
>> implementation-defined.
>>> I think you might also get a warning for the opposite direction.
>>
>> Implementation-defined.
>>
>> The standard defines semantics of valid void * -> intptr_t, uintptr_t ->
>> void *: you get your original pointer back ("will compare equal").
>>
>> The standard is silent on converting integer type to pointer type and
>> back. Doesn't matter. No sane implementation screws that up.
>
> That's correct. int or long to pointer and back normally works.
>
> But the compiler does not know whether the two conversions are ordered
> integer to pointer - pointer to integer or
> pointer to integer - integer to pointer.
>
> Here is a short example using int instead of long,
> so it will show the warnings on any linux host:
>
> int ptr2int(void *ptr)
> {
> return (int)ptr;
> }
>
> void *int2ptr(int i)
> {
> return (void *)i;
> }
>
> gcc -Wall -c intptr.c
> intptr.c: In function ‘ptr2int’:
> intptr.c:3: warning: cast from pointer to integer of different size
> intptr.c: In function ‘int2ptr’:
> intptr.c:8: warning: cast to pointer from integer of different size
>
> The same kind of warnings occur with the current qemu code when
> I cross compile using Debian's amd64-mingw32msvc-gcc.
>
> So the patch is needed for w64. For all other currently known
> host architectures, it is not needed, but nevertheless it will
> make the intention of the code clearer (as was pointed out in
> an earlier mail on this subject).
>
> Please apply the patch to qemu master.
> If needed, the patch's subject can be modified
> (w64: Fix conversions from pointer to int and vice versa)
>
> Thanks,
> Stefan


No more comments? There was no nack, and for w64 the patch
(or another solution) is needed.

What can I do to get this patch committed to QEMU git master?

Regards,
Stefan W.