exec: don't use void* in pointer arithmetic in headers

[PATCH v2] exec: don't use void* in pointer arithmetic in headers

Posted by Roman Kiryanov 6 months, 1 week ago

void* pointer arithmetic is a GCC extentension
which could not be available in other build
tools (e.g. C++). This changes removes this
assumption.

Google-Bug-Id: 331190993
Change-Id: I5a064853429f627c17a9213910811dea4ced6174
Signed-off-by: Roman Kiryanov <rkir@google.com>
---
v2: renamed from "use char* for pointer arithmetic"
    and removed all explicit extra cast with
    one typedef in memory.h.

 include/exec/memory.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/include/exec/memory.h b/include/exec/memory.h
index b1713f30b8..b616338f05 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -2795,8 +2795,10 @@ MemTxResult address_space_write_rom(AddressSpace *as, hwaddr addr,
 #define ARG1_DECL    AddressSpace *as
 #include "exec/memory_ldst_phys.h.inc"
 
+typedef uint8_t *MemoryRegionCachePtr;
+
 struct MemoryRegionCache {
-    void *ptr;
+    MemoryRegionCachePtr ptr;
     hwaddr xlat;
     hwaddr len;
     FlatView *fv;
-- 
2.45.2.741.gdbec12cfda-goog

Re: [PATCH v2] exec: don't use void* in pointer arithmetic in headers

Posted by Paolo Bonzini 6 months, 1 week ago

On 6/20/24 22:16, Roman Kiryanov wrote:
> void* pointer arithmetic is a GCC extentension
> which could not be available in other build
> tools (e.g. C++). This changes removes this
> assumption.
> 
> Google-Bug-Id: 331190993
> Change-Id: I5a064853429f627c17a9213910811dea4ced6174
> Signed-off-by: Roman Kiryanov <rkir@google.com>
> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> v2: renamed from "use char* for pointer arithmetic"
>      and removed all explicit extra cast with
>      one typedef in memory.h.
> 
>   include/exec/memory.h | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index b1713f30b8..b616338f05 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -2795,8 +2795,10 @@ MemTxResult address_space_write_rom(AddressSpace *as, hwaddr addr,
>   #define ARG1_DECL    AddressSpace *as
>   #include "exec/memory_ldst_phys.h.inc"
>   
> +typedef uint8_t *MemoryRegionCachePtr;
> +
>   struct MemoryRegionCache {
> -    void *ptr;
> +    MemoryRegionCachePtr ptr;

Just "uint8_t *ptr" is enough; thanks for testing that it's enough.

Queued for the next pull request, thanks.

Paolo


>       hwaddr xlat;
>       hwaddr len;
>       FlatView *fv;

Re: [PATCH v2] exec: don't use void* in pointer arithmetic in headers

Posted by Philippe Mathieu-Daudé 6 months, 1 week ago

Hi Roman,

On 20/6/24 22:16, Roman Kiryanov wrote:
> void* pointer arithmetic is a GCC extentension
> which could not be available in other build
> tools (e.g. C++). This changes removes this
> assumption.
> 
> Google-Bug-Id: 331190993

Again [*] I'm trying to figure what this tag is, no hit on
https://issuetracker.google.com/issues?q=id:331190993; is
this useful to commit it in the mainstream repository? So
far it is confusing me.

[*] 
https://lore.kernel.org/qemu-devel/e865d8e3-e768-4b1f-86d3-aeabe8f1d511@linaro.org/

> Change-Id: I5a064853429f627c17a9213910811dea4ced6174

Ditto, not useful.

Per 
https://lore.kernel.org/qemu-devel/4b3d0472-8b06-403a-9ab8-553aa858fb7f@redhat.com/
I wonder if this deserves a:
Suggested-by: Paolo Bonzini <pbonzini@redhat.com>

> Signed-off-by: Roman Kiryanov <rkir@google.com>
> ---
> v2: renamed from "use char* for pointer arithmetic"
>      and removed all explicit extra cast with
>      one typedef in memory.h.
> 
>   include/exec/memory.h | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index b1713f30b8..b616338f05 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -2795,8 +2795,10 @@ MemTxResult address_space_write_rom(AddressSpace *as, hwaddr addr,
>   #define ARG1_DECL    AddressSpace *as
>   #include "exec/memory_ldst_phys.h.inc"
>   
> +typedef uint8_t *MemoryRegionCachePtr;
> +

What about:

   typedef uint8_t UnstructuredData;

>   struct MemoryRegionCache {
> -    void *ptr;
> +    MemoryRegionCachePtr ptr;

and:

       UnstructuredData *ptr;

?

>       hwaddr xlat;
>       hwaddr len;
>       FlatView *fv;

Re: [PATCH v2] exec: don't use void* in pointer arithmetic in headers

Posted by Roman Kiryanov 6 months ago

On Thu, Jun 20, 2024 at 11:42 PM Philippe Mathieu-Daudé
<philmd@linaro.org> wrote:
>
> Hi Roman,
>
> On 20/6/24 22:16, Roman Kiryanov wrote:
> > Google-Bug-Id: 331190993
>
> Again [*] I'm trying to figure what this tag is, no hit on
> https://issuetracker.google.com/issues?q=id:331190993; is
> this useful to commit it in the mainstream repository? So
> far it is confusing me.
>
> [*]
> https://lore.kernel.org/qemu-devel/e865d8e3-e768-4b1f-86d3-aeabe8f1d511@linaro.org/
>
> > Change-Id: I5a064853429f627c17a9213910811dea4ced6174
>
> Ditto, not useful.

I am sorry, Philippe, I missed your message. If you prefer us to remove
Google specific tags from our commits, we will remove them.

Regards,
Roman.