| Message ID | 20200330192157.1335-1-julien@xen.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | Fix build with using OCaml 4.06.1 and -safe-string | expand |
Julien Grall writes ("[PATCH 0/8] Fix build with using OCaml 4.06.1 and -safe-string"):
> This series is meant to solve the build issue reported by Dario when
> using recent version of OCaml and -safe-string.
Thanks. I have reviewed the C tools parts here. I think the ocaml
parts ought to have a review from someone familiar with the ocaml FFI.
> I took the opportunity to harden a bit more the code by using const more
> often.
I approve.
Perhaps we should start building our C code with -Wwrite-strings,
which makes "" have type const char* ? Result would be a giant
constification patch, probably.
Ian.
Hi Ian, On 31/03/2020 12:17, Ian Jackson wrote: > Julien Grall writes ("[PATCH 0/8] Fix build with using OCaml 4.06.1 and -safe-string"): >> This series is meant to solve the build issue reported by Dario when >> using recent version of OCaml and -safe-string. > > Thanks. I have reviewed the C tools parts here. I think the ocaml > parts ought to have a review from someone familiar with the ocaml FFI. > >> I took the opportunity to harden a bit more the code by using const more >> often. > > I approve. > > Perhaps we should start building our C code with -Wwrite-strings, > which makes "" have type const char* ? Result would be a giant > constification patch, probably. So I thought I would give a try and see how far I can go: * hypervisor (xen): It is fairly easy to convert, although this is touching code that was imported from other projects (such as acpica). I need to have a look at whether other projects fixed there code and we can backport. * libxc: This is pretty trivial, I will send a patch for it * libxl: This is where it is getting tricky, the main issue is the flexarray framework as we would use it with string (now const char *). I thought we could make the interface const, but it looks like there are a couple of places where we need to modify the content (such as in libxl_json.c). I am not sure yet how to deal with it. In any case, even if we can't use -Wwrite-strings, I can still send patches to use const in more places. Cheers,
Hi, Gentle ping. I am missing reviews for the OCaml part. Cheers, On 30/03/2020 20:21, Julien Grall wrote: > From: Julien Grall <jgrall@amazon.com> > > Hi all, > > This series is meant to solve the build issue reported by Dario when > using recent version of OCaml and -safe-string. > > I took the opportunity to harden a bit more the code by using const more > often. > > This series was only build tested. > > Cheers, > > Julien Grall (8): > xen/guest_access: Harden copy_to_guest_offset to prevent const dest > operand > xen/public: sysctl: set_parameter.params and debug.keys should be > const > tools/libxc: misc: Mark const the parameter 'keys' of > xc_send_debug_keys() > tools/libxc: misc: Mark const the parameter 'params' of > xc_set_parameters() > tools/ocaml: libxc: Check error return in stub_xc_vcpu_context_get() > tools/ocaml: libxb: Harden stub_header_of_string() > tools/ocaml: libxb: Avoid to use String_val() when value is bytes > tools/ocaml: Fix stubs build when OCaml has been compiled with > -safe-string > > tools/libxc/include/xenctrl.h | 4 ++-- > tools/libxc/xc_misc.c | 8 ++++---- > tools/libxc/xc_private.h | 8 ++++++++ > tools/ocaml/libs/xb/xenbus_stubs.c | 6 +++--- > tools/ocaml/libs/xb/xs_ring_stubs.c | 12 ++++++++++-- > tools/ocaml/libs/xc/xenctrl_stubs.c | 6 ++++-- > xen/include/asm-arm/guest_access.h | 2 +- > xen/include/asm-x86/guest_access.h | 2 +- > xen/include/public/sysctl.h | 4 ++-- > 9 files changed, 35 insertions(+), 17 deletions(-) >
The changes in the OCaml C stubs look good to me. They don't really touch the interface but are mostly concerned with types on the C side by adding casts, const, and so on. The extended error handling is an improvement. -- Christian
Hi Christian, On 16/04/2020 14:25, Christian Lindig wrote: > > The changes in the OCaml C stubs look good to me. They don't really touch the interface but are mostly concerned with types on the C side by adding casts, const, and so on. The extended error handling is an improvement. Thank you for the review! I will commit the rest of the series. As an aside, the acked-by was adding as part of the signature. Not sure whether this is intentional but some e-mail clients are hiding the signature so the acked-by can be easily missed. Cheers,
From: Julien Grall <jgrall@amazon.com> Hi all, This series is meant to solve the build issue reported by Dario when using recent version of OCaml and -safe-string. I took the opportunity to harden a bit more the code by using const more often. This series was only build tested. Cheers, Julien Grall (8): xen/guest_access: Harden copy_to_guest_offset to prevent const dest operand xen/public: sysctl: set_parameter.params and debug.keys should be const tools/libxc: misc: Mark const the parameter 'keys' of xc_send_debug_keys() tools/libxc: misc: Mark const the parameter 'params' of xc_set_parameters() tools/ocaml: libxc: Check error return in stub_xc_vcpu_context_get() tools/ocaml: libxb: Harden stub_header_of_string() tools/ocaml: libxb: Avoid to use String_val() when value is bytes tools/ocaml: Fix stubs build when OCaml has been compiled with -safe-string tools/libxc/include/xenctrl.h | 4 ++-- tools/libxc/xc_misc.c | 8 ++++---- tools/libxc/xc_private.h | 8 ++++++++ tools/ocaml/libs/xb/xenbus_stubs.c | 6 +++--- tools/ocaml/libs/xb/xs_ring_stubs.c | 12 ++++++++++-- tools/ocaml/libs/xc/xenctrl_stubs.c | 6 ++++-- xen/include/asm-arm/guest_access.h | 2 +- xen/include/asm-x86/guest_access.h | 2 +- xen/include/public/sysctl.h | 4 ++-- 9 files changed, 35 insertions(+), 17 deletions(-)