Re: [PATCH v2] Drop more useless casts from void * to pointer

[Markus Armbruster <armbru@redhat.com>]

BALATON Zoltan <balaton@eik.bme.hu> writes:

> 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.

And that's perfectly okay!