Re: [Xen-devel] [PATCH 1/2] make xen ocaml safe-strings compliant

[Michael Young <m.a.young@xxxxxxxxxxxx>]

On Mon, 12 Mar 2018, Christian Lindig wrote:

> >  The problem with the old patch is illustrated by the following section
> >  from the old patch for tools/ocaml/xenstored/utils.ml
> >  @@ -85,7 +85,7 @@ let create_unix_socket name =
> >   let read_file_single_integer filename =
> >          let fd = Unix.openfile filename [ Unix.O_RDONLY ] 0o640 in
> >          let buf = String.make 20 (char_of_int 0) in
> >  -       let sz = Unix.read fd buf 0 20 in
> >  +       let sz = Unix.read fd (Bytes.of_string buf) 0 20 in
> >          Unix.close fd;
> >          int_of_string (String.sub buf 0 sz)
> >
> >  where the patch makes Unix.read write to a Bytes copy of buf and buf
> >  itself is unchanged, so int_of_string sees a string of null characters
> >  rather than a string to convert into a number.
> Good analysis. (Bytes.of_string buf) created a buffer for the result from 
> read() for which we have no handle.
> Reviewing the new patch I believe it is sound. The (new) signature of 
> read_mmap is
> >  val read_mmap : backend_mmap -> 'a -> bytes -> int -> int
> The new implementation is below - argument s used to be a string value and is 
> now a bytes value. I would suggest to reflect this in the names (using b 
> instead of s) as this is about conversion between strings and bytes.
> >    let read_mmap back con s len =
> >  -      let rd = Xs_ring.read back.mmap s len in
> >  +      let stmp = String.make len (char_of_int 0) in
> >  +      let rd = Xs_ring.read back.mmap stmp len in
> >  +      Bytes.blit_string stmp 0 s 0 rd;
> >     back.work_again <- (rd > 0);
> >     if rd > 0 then
> >      back.eventchn_notify ();
> Below are the functions that changed their type and where this also should be 
> considered:
> >  -val read_fd : backend_fd -> 'a -> string -> int -> int
> >  -val read_mmap : backend_mmap -> 'a -> string -> int -> int
> >  -val read : t -> string -> int -> int
> >  -val write_fd : backend_fd -> 'a -> string -> int -> int
> >  +val read_fd : backend_fd -> 'a -> bytes -> int -> int
> >  +val read_mmap : backend_mmap -> 'a -> bytes -> int -> int
> >  +val read : t -> bytes -> int -> int
> >  +val write_fd : backend_fd -> 'a -> bytes -> int -> int
> -- Christian
Here is version 4 of the patch where I have replaced the uses of s with b 
where the patch changes it from string to bytes. I have also removed the 
two trailing spaces and changed stmp back to s.
        Michael Young

Attachment: 0001-make-xen-ocaml-safe-strings-compliant.patch
Description: Text document

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel