ocaml / ocaml

The core OCaml system: compilers, runtime system, base libraries
https://ocaml.org
Other
5.19k stars 1.06k forks source link

Clarify how to reassign the pointer of a custom block #13108

Closed dbuenzli closed 1 week ago

dbuenzli commented 3 weeks ago

It seems that Data_custom_val(v) is not assignable and the manual doesn't seem to mention (I would have expected here or here) in which field the custom pointer is stored.

For now I'm using Store_field (cval, 0, (value)NULL) Store_field (cval, 1, (value)NULL) but perhaps a Store_data_custom_val operation could be added ?

lthls commented 3 weeks ago

I'm not sure it makes sense as a macro. Data_custom_val returns a void* pointer, so users are expected to cast it to the right type before manipulating the contents:

  struct my_custom * data = Data_custom_val(v);
  data->first = 0;
  data->second = NULL;

This is tersely documented here. Maybe what's missing is that Data_custom_val doesn't return the contents itself, but a pointer inside the custom block that can be used to modify the contents. For example, the snippet above assumes that a struct with two fields is embedded inside the custom block (without indirection), and sets the fields to zero.

dbuenzli commented 3 weeks ago

That's not my point. I want to set the pointer returned by Data_custom_val to NULL.

My use case is that I have a custom block with a pointer and a finalizer. However I may know before that I want to free the pointer. In which case I free the pointer and update the value returned by Data_custom_val to NULL. If I don't update Data_custom_val that will lead to a double free when the custom block is finalized.

lthls commented 3 weeks ago

You can't set the pointer returned by Data_custom_val(v) to NULL, because it isn't stored anywhere. It's basically &(v->snd), where snd is the second field of a C struct. Either you are storing a full struct in the custom block (| header | custom_ops | struct_field1 | struct_field2 | ... |), in which case the individual fields can be freed but the struct itself is not malloc'ed no there's nothing to free, or you are storing a pointer (| header | custom_ops | pointer_to_struct |) in which case Data_custom_val gives you a pointer to the pointer, and you should be able to set the pointer to NULL normally *((my_struct **)(Data_custom_val(v))) = NULL.

dbuenzli commented 3 weeks ago

Ah I see. Thanks. I think I read too much in the wording here:

Blocks with tag Custom_tag contain both arbitrary user data and a pointer to a C struct, with type struct custom_operations, that associates user-provided finalization, comparison, hashing, serialization and deserialization functions to this block.

Somehow I thought this was a two field block. One field for the custom ops and one field for the pointer.

gasche commented 3 weeks ago

Maybe the documentation could be clarified into:

Blocks with tag Custom_tag contain a pointer to "custom operations", followed by zero, one or several words of raw data. The custom operations are stored in a C struct of type custom_operations and contain user-provided finalization, comparison, hashing, serialization and deserialization functions to this custom block.

dbuenzli commented 3 weeks ago

I can do that at some point. Just reopening so that I don't forget.