Improve OCaml 5 compatibility #24
Conversation
lindig
commented
Mar 18, 2026
lindig
commented
- OCaml 5 must avoid naked pointers which the code is currently using in one of the bindings.
- Reformat code using the latest OCamlformat that we are using.
- Release the runtime lock when calling C functions.
| caml_acquire_runtime_system (); | ||
|
|
||
| if (rc == -1) | ||
| caml_failwith (__func__); |
There was a problem hiding this comment.
Should we use uerror here and similar places? Otherwise we lose errno
| if (global_xce == NULL) | ||
| caml_failwith (strerror (errno)); | ||
| if (xce == NULL) | ||
| caml_failwith (__func__); |
Signed-off-by: Christian Lindig <christian.lindig@citrix.com>
Signed-off-by: Christian Lindig <christian.lindig@citrix.com>
The FFI call must not use *val macros, so pull them in front of the actual call. Signed-off-by: Christian Lindig <christian.lindig@citrix.com>
lindig
force-pushed
the
private/christianlin/CP-311786
branch
from
669642c to
761a77c
Compare
| addr = mmap (NULL, length, c_pflag, c_mflag, Int_val (fd), | ||
| Int_val (offset)); | ||
| int c_fd = Int_val (fd); | ||
| int c_offset = Int_val (offset); |
There was a problem hiding this comment.
Unrelated to this PR, but the types used here appear to be the wrong one. This shouldn't be an int, but a long, and Long_val should've been used. They should match what the underlying API takes as a parameter (in this case off_t, which can be either 32-bit or 64-bit).
A C int cannot fully represent the values of an OCaml int on 64-bit platforms (which is the only one we support). On x86-64 Linux a C int is 32-bits , and an OCaml int is 63 bits. So most of the time the Int_val macro is the wrong one to use, unless the underlying C API takes an int argument (e.g. for a file descriptor it is fine, because of ulimit).
There was a problem hiding this comment.
This fixes the OCaml 5 issue. There are other preexisting issues with the bindings, but we don't need to fix those right now (e.g. they probably shouldn't output things to stderr on error, that is the caller's responsibility; the wrong Int_val vs Long_val macro being used, etc.)
|
I can remove the |
Signed-off-by: Christian Lindig <christian.lindig@citrix.com>
OCaml 5 is not compatible with naked pointers: C pointers returned as an OCaml value. Instead, such a pointer must be wrapped as an OCaml custom value. This patch: * returns the event channel handle as an OCaml custom value * releases the OCaml runtime lock over calls into the library * avoids Val_* macros in argument positions of such calls * removese the global "global_xce" and relies on a finaliser to close event channels * simpliefies error handling Signed-off-by: Christian Lindig <christian.lindig@citrix.com>
Don't use perror() in a library; leave it to the client. Signed-off-by: Christian Lindig <christian.lindig@citrix.com>
lindig
force-pushed
the
private/christianlin/CP-311786
branch
from
a1e77ec to
892068d
Compare
3 participants
