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(-)

-- 
2.17.1

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,

-- 
Julien Grall

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(-)
> 

-- 
Julien Grall

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

-- 
Acked-by: Christian Lindig <christian.lindig@citrix.com>

________________________________________
From: Julien Grall <julien@xen.org>
Sent: 16 April 2020 12:25
To: xen-devel@lists.xenproject.org; Christian Lindig; David Scott
Cc: dfaggioli@suse.com; Julien Grall; Stefano Stabellini; Volodymyr Babchuk; Jan Beulich; Andrew Cooper; Wei Liu; Roger Pau Monne; George Dunlap; Ian Jackson
Subject: Re: [PATCH 0/8] Fix build with using OCaml 4.06.1 and -safe-string

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(-)
>

--
Julien Grall

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,

-- 
Julien Grall