|
From: | BALATON Zoltan |
Subject: | Re: [PATCH v2] Drop more useless casts from void * to pointer |
Date: | Wed, 23 Nov 2022 17:29:27 +0100 (CET) |
On Wed, 23 Nov 2022, Markus Armbruster wrote:
Daniel P. Berrangé <berrange@redhat.com> writes:On Wed, Nov 23, 2022 at 02:51:49PM +0100, BALATON Zoltan wrote:On Wed, 23 Nov 2022, Markus Armbruster wrote:Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Laurent Vivier <laurent@vivier.eu> --- v2: * PATCH 1+2 merged as commit 0a553c12c7 and 3f7febc937 * PATCH 3 change to util/coroutine-ucontext.c dropped [Laurent] bsd-user/elfload.c | 2 +- contrib/plugins/cache.c | 8 ++++---- contrib/vhost-user-blk/vhost-user-blk.c | 2 +- hw/core/qdev-clock.c | 2 +- hw/hyperv/vmbus.c | 2 +- hw/net/cadence_gem.c | 2 +- hw/net/virtio-net.c | 2 +- hw/nvme/ctrl.c | 4 ++-- hw/rdma/vmw/pvrdma_cmd.c | 9 +++------ hw/rdma/vmw/pvrdma_qp_ops.c | 6 +++--- hw/virtio/virtio-iommu.c | 3 +-- linux-user/syscall.c | 2 +- target/i386/hax/hax-all.c | 2 +- tests/tcg/aarch64/system/semiheap.c | 4 ++-- util/vfio-helpers.c | 2 +- 15 files changed, 24 insertions(+), 28 deletions(-) diff --git a/bsd-user/elfload.c b/bsd-user/elfload.c index f8edb22f2a..fbcdc94b96 100644 --- a/bsd-user/elfload.c +++ b/bsd-user/elfload.c @@ -156,7 +156,7 @@ static abi_ulong copy_elf_strings(int argc, char **argv, void **page, --p; --tmp; --len; if (--offset < 0) { offset = p % TARGET_PAGE_SIZE; - pag = (char *)page[p / TARGET_PAGE_SIZE]; + pag = page[p / TARGET_PAGE_SIZE];I think arithmetic on void pointer was undefined at least in the past so some compilers may warn for it but not sure if this is still the case for the compilers we care about. Apparently not if this now compiles but that explains why this cast was not useless.I don't think so :) @pag is char *. @page is void **. page[p / TARGET_PAGE_SIZE] is void *. No need to cast to char * before assigning to @pag.
You are right. Although I'm not sure what page[] counts as because it adds an offset to the pointer but [] is higher priority than (type) cast so it does not matter and that cast is not needed here then. Maybe I should be more attentive to details but I did not take the time to look at it more carefully. I did not say we should keep the cast anyway (considering only gcc and clang are targeted), I was just trying to understand why it might have been there in the first place.
Regards, BALATON Zoltan
No pointer arithmetic so far. There's some further down: pag + offset. @pag is char * before and after my patch.Found some more info on this here: https://stackoverflow.com/questions/3523145/pointer-arithmetic-for-void-pointer-in-cQEMU explicitly only targets GCC + Clang, so portability to other compilers is not required.Correct. We do arithmentic with void * in many places already. If we cared for portability to other compilers, we'd enable '-Wpointer-arith' Warn about anything that depends on the "size of" a function type or of 'void'. GNU C assigns these types a size of 1, for convenience in calculations with 'void *' pointers and pointers to functions. In C++, warn also when an arithmetic operation involves 'NULL'. This warning is also enabled by '-Wpedantic'. But we don't.
[Prev in Thread] | Current Thread | [Next in Thread] |