| Message ID | 1298484556-5517-1-git-send-email-weil@mail.berlios.de |
|---|---|
| State | Accepted |
| Headers | show |
Stefan Weil <weil@mail.berlios.de> wrote: > 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 *). > > Signed-off-by: Stefan Weil <weil@mail.berlios.de> migration-{tcp,unix}.c conflict with my series. If Anthony don't pick them before my next respin, I will include that two files. Thanks, Juan.
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? > @@ -295,7 +295,7 @@ static int qemu_event_init(void) > goto fail; > } > qemu_set_fd_handler2(fds[0], NULL, qemu_event_read, NULL, > - (void *)(unsigned long)fds[0]); > + (void *)(intptr_t)fds[0]); > > io_thread_fd = fds[1]; > return 0; Why can't you cast straight to void *? [More of the same snipped...]
On 02/24/2011 08:21 AM, Markus Armbruster wrote:
> Why can't you cast straight to void *?
"warning: cast from pointer to integer of different size", and similarly
in the other direction.
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 (the behaviour is undefined if the integer is too small). I think you might also get a warning for the opposite direction. Kevin
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.
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
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.
Thanks, applied. On Sun, Mar 20, 2011 at 2:07 PM, Stefan Weil <weil@mail.berlios.de> wrote: > 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. > >
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]; @@ -295,7 +295,7 @@ static int qemu_event_init(void) goto fail; } qemu_set_fd_handler2(fds[0], NULL, qemu_event_read, NULL, - (void *)(unsigned long)fds[0]); + (void *)(intptr_t)fds[0]); io_thread_fd = fds[1]; return 0; @@ -316,7 +316,7 @@ static void dummy_signal(int sig) */ static void sigfd_handler(void *opaque) { - int fd = (unsigned long) opaque; + int fd = (intptr_t)opaque; struct qemu_signalfd_siginfo info; struct sigaction action; ssize_t len; @@ -358,7 +358,7 @@ static int qemu_signalfd_init(sigset_t mask) fcntl_setfl(sigfd, O_NONBLOCK); qemu_set_fd_handler2(sigfd, NULL, sigfd_handler, NULL, - (void *)(unsigned long) sigfd); + (void *)(intptr_t)sigfd); return 0; } diff --git a/migration-tcp.c b/migration-tcp.c index b55f419..e8dff9d 100644 --- a/migration-tcp.c +++ b/migration-tcp.c @@ -139,7 +139,7 @@ static void tcp_accept_incoming_migration(void *opaque) { struct sockaddr_in addr; socklen_t addrlen = sizeof(addr); - int s = (unsigned long)opaque; + int s = (intptr_t)opaque; QEMUFile *f; int c; @@ -194,7 +194,7 @@ int tcp_start_incoming_migration(const char *host_port) goto err; qemu_set_fd_handler2(s, NULL, tcp_accept_incoming_migration, NULL, - (void *)(unsigned long)s); + (void *)(intptr_t)s); return 0; diff --git a/migration-unix.c b/migration-unix.c index 57232c0..8b967f2 100644 --- a/migration-unix.c +++ b/migration-unix.c @@ -147,7 +147,7 @@ static void unix_accept_incoming_migration(void *opaque) { struct sockaddr_un addr; socklen_t addrlen = sizeof(addr); - int s = (unsigned long)opaque; + int s = (intptr_t)opaque; QEMUFile *f; int c; @@ -204,7 +204,7 @@ int unix_start_incoming_migration(const char *path) } qemu_set_fd_handler2(sock, NULL, unix_accept_incoming_migration, NULL, - (void *)(unsigned long)sock); + (void *)(intptr_t)sock); return 0; diff --git a/qemu-char.c b/qemu-char.c index bd4e944..cad35d7 100644 --- a/qemu-char.c +++ b/qemu-char.c @@ -1376,7 +1376,7 @@ static CharDriverState *qemu_chr_open_pp(QemuOpts *opts) #if defined(__FreeBSD__) || defined(__FreeBSD_kernel__) || defined(__DragonFly__) static int pp_ioctl(CharDriverState *chr, int cmd, void *arg) { - int fd = (int)(long)chr->opaque; + int fd = (int)(intptr_t)chr->opaque; uint8_t b; switch(cmd) { @@ -1422,7 +1422,7 @@ static CharDriverState *qemu_chr_open_pp(QemuOpts *opts) return NULL; chr = qemu_mallocz(sizeof(CharDriverState)); - chr->opaque = (void *)(long)fd; + chr->opaque = (void *)(intptr_t)fd; chr->chr_write = null_chr_write; chr->chr_ioctl = pp_ioctl; return chr;
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 *). 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(-)