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

[Blue Swirl]

Thanks, applied.

On Sun, Mar 20, 2011 at 2:07 PM, Stefan Weil <address@hidden> wrote:
> Am 24.02.2011 20:57, schrieb Stefan Weil:
>>
>> Am 24.02.2011 11:11, schrieb Markus Armbruster:
>>>
>>> Kevin Wolf <address@hidden> writes:
>>>>
>>>> Am 24.02.2011 08:21, schrieb Markus Armbruster:
>>>>>
>>>>> Stefan Weil <address@hidden> 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 <address@hidden>
>>>>>> ---
>>>>>> 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.
>
>