On Tue, Mar 08, 2022 at 09:05:27AM +0000, Peter Maydell wrote: > On Mon, 7 Mar 2022 at 22:52, Michael S. Tsirkin <m...@redhat.com> wrote: > > > > On Mon, Mar 07, 2022 at 05:13:16PM +0000, Peter Maydell wrote: > > > Also fails on cross-win64-system: > > > > > > https://gitlab.com/qemu-project/qemu/-/jobs/2172339938 > > > > > > ../hw/virtio/virtio.c: In function > > > 'qmp_x_query_virtio_vhost_queue_status': > > > ../hw/virtio/virtio.c:4358:30: error: cast from pointer to integer of > > > different size [-Werror=pointer-to-int-cast] > > > 4358 | status->desc = (uint64_t)(unsigned long)hdev->vqs[queue].desc; > > > | ^ > > > ../hw/virtio/virtio.c:4359:31: error: cast from pointer to integer of > > > different size [-Werror=pointer-to-int-cast] > > > 4359 | status->avail = (uint64_t)(unsigned long)hdev->vqs[queue].avail; > > > | ^ > > > ../hw/virtio/virtio.c:4360:30: error: cast from pointer to integer of > > > different size [-Werror=pointer-to-int-cast] > > > 4360 | status->used = (uint64_t)(unsigned long)hdev->vqs[queue].used; > > > | ^ > > > cc1: all warnings being treated as errors > > > I dropped these for now but I really question the value of this warning, > > as you can see the reason we have the buggy cast to unsigned long > > is because someone wanted to shut up the warning on a 32 bit system. > > > > Now, I could maybe get behind this if it simply warned about a cast that > > loses information (cast to a smaller integer) or integer/pointer cast > > that does not go through uintptr_t without regard to size. > > This *is* warning about losing information. On 64-bit Windows > pointers are 64 bits but 'long' is 32 bits, so the path > pointer -> long -> uint64_t drops the top half of the pointer. > > thanks > -- PMM
Yes obviously. My point is that this: (uint64_t)hdev->vqs[queue].avail is always harmless but it warns on a 32 bit system. And someone trying to fix that *is* what resulted in (uint64_t)(unsigned long)hdev->vqs[queue].avail IOW I don't really see how (uint64_t)(uintptr_t)hdev->vqs[queue].avail is better than (uint64_t)hdev->vqs[queue].avail except as a way to say "yes I do intend to cast pointer to integer here, I did not forget to dereference the pointer". But if that latter is what gcc is trying to warn about, then it should just warn about any cast to integer except to uintptr_t, without respect to size. -- MST
