Re: [PATCH v4 01/11] tools/ocaml/libs/{xb, mmap}: use Data_abstract_val wrapper

[Edwin Torok <edwin.torok@xxxxxxxxx>]

> On 16 Dec 2022, at 22:40, Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx> wrote:
> 
> On 16/12/2022 6:25 pm, Edwin Török wrote:
>> diff --git a/tools/ocaml/libs/mmap/mmap_stubs.h 
>> b/tools/ocaml/libs/mmap/mmap_stubs.h
>> index 65e4239890..66f18d4406 100644
>> --- a/tools/ocaml/libs/mmap/mmap_stubs.h
>> +++ b/tools/ocaml/libs/mmap/mmap_stubs.h
>> @@ -30,4 +30,8 @@ struct mmap_interface
>> int len;
>> };
>> 
>> +#ifndef Data_abstract_val
>> +#define Data_abstract_val(x) ((void*)(value*)(x))
> 
> ((void *)(x))
> 
> I take it this has come from the Ocaml headers?  The cast to (value *)
> in the middle is entirely cancelled out.
> 
> Can be fixed on commit.
> 


In the OCaml headers it looks like:
+#define Data_abstract_val(v) ((void*) Op_val(v))

where Op_val is:
+#define Op_val(x) ((value *) (x))

However I haven't realized that Op_val has been in OCaml for a long time (at 
least 1995), so we can safely use it, and do this instead:

diff --git a/tools/ocaml/libs/mmap/mmap_stubs.h 
b/tools/ocaml/libs/mmap/mmap_stubs.h
index 65e4239890..6c33f14138 100644
--- a/tools/ocaml/libs/mmap/mmap_stubs.h
+++ b/tools/ocaml/libs/mmap/mmap_stubs.h
@@ -30,4 +30,8 @@ struct mmap_interface
    int len;
 };

+#ifndef Data_abstract_val
+#define Data_abstract_val(v) ((void*) Op_val(v))
+#endif
+
 #endif


There is an updated commit here for convenience: 
https://github.com/edwintorok/xen/commit/9855ed237f3f85ac30972bfd0a601c6746ba2353

Best regards,
--Edwin