* [PATCH 1/2] make xen ocaml safe-strings compliant @ 2018-01-30 22:55 Michael Young 2018-02-06 16:49 ` Wei Liu 2018-02-08 17:49 ` Dario Faggioli 0 siblings, 2 replies; 18+ messages in thread From: Michael Young @ 2018-01-30 22:55 UTC (permalink / raw) To: xen-devel Cc: Marcello Seri, Wei Liu, Christian Lindig, John Thomson, David Scott Xen built with ocaml 4.06 gives errors such as Error: This expression has type bytes but an expression was expected of type string as Byte and safe-strings which were introduced in 4.02 are the default in 4.06. This patch which is partly by Richard W.M. Jones of Red Hat from https://bugzilla.redhat.com/show_bug.cgi?id=1526703 fixes these issues. Signed-off-by: Michael Young <m.a.young@durham.ac.uk> Reviewed-by: Christian Lindig <christian.lindig@citrix.com> --- tools/ocaml/libs/xb/xb.ml | 6 +++--- tools/ocaml/libs/xc/xenctrl.ml | 2 +- tools/ocaml/xenstored/logging.ml | 22 +++++++++++----------- tools/ocaml/xenstored/stdext.ml | 2 +- tools/ocaml/xenstored/utils.ml | 18 +++++++++--------- 5 files changed, 25 insertions(+), 25 deletions(-) diff --git a/tools/ocaml/libs/xb/xb.ml b/tools/ocaml/libs/xb/xb.ml index 50944b5fd6..aa2cf98223 100644 --- a/tools/ocaml/libs/xb/xb.ml +++ b/tools/ocaml/libs/xb/xb.ml @@ -84,7 +84,7 @@ let read_mmap back con s len = let read con s len = match con.backend with - | Fd backfd -> read_fd backfd con s len + | Fd backfd -> read_fd backfd con (Bytes.of_string s) len | Xenmmap backmmap -> read_mmap backmmap con s len let write_fd back con s len = @@ -98,7 +98,7 @@ let write_mmap back con s len = let write con s len = match con.backend with - | Fd backfd -> write_fd backfd con s len + | Fd backfd -> write_fd backfd con (Bytes.of_string s) len | Xenmmap backmmap -> write_mmap backmmap con s len (* NB: can throw Reconnect *) @@ -147,7 +147,7 @@ let input con = | NoHdr (i, buf) -> (* we complete the partial header *) if sz > 0 then - String.blit s 0 buf (Partial.header_size () - i) sz; + String.blit s 0 (Bytes.of_string buf) (Partial.header_size () - i) sz; con.partial_in <- if sz = i then HaveHdr (Partial.of_string buf) else NoHdr (i - sz, buf) ); diff --git a/tools/ocaml/libs/xc/xenctrl.ml b/tools/ocaml/libs/xc/xenctrl.ml index 9116aa222c..31b13f4bbd 100644 --- a/tools/ocaml/libs/xc/xenctrl.ml +++ b/tools/ocaml/libs/xc/xenctrl.ml @@ -292,7 +292,7 @@ external marshall_core_header: core_header -> string = "stub_marshall_core_heade (* coredump *) let coredump xch domid fd = let dump s = - let wd = Unix.write fd s 0 (String.length s) in + let wd = Unix.write fd (Bytes.of_string s) 0 (String.length s) in if wd <> String.length s then failwith "error while writing"; in diff --git a/tools/ocaml/xenstored/logging.ml b/tools/ocaml/xenstored/logging.ml index 0c0d03d0c4..d24abf8a3a 100644 --- a/tools/ocaml/xenstored/logging.ml +++ b/tools/ocaml/xenstored/logging.ml @@ -60,11 +60,11 @@ type logger = let truncate_line nb_chars line = if String.length line > nb_chars - 1 then let len = max (nb_chars - 1) 2 in - let dst_line = String.create len in - String.blit line 0 dst_line 0 (len - 2); - dst_line.[len-2] <- '.'; - dst_line.[len-1] <- '.'; - dst_line + let dst_line = Bytes.create len in + Bytes.blit_string line 0 dst_line 0 (len - 2); + Bytes.set dst_line (len-2) '.'; + Bytes.set dst_line (len-1) '.'; + Bytes.to_string dst_line else line let log_rotate ref_ch log_file log_nb_files = @@ -252,13 +252,13 @@ let string_of_access_type = function *) let sanitize_data data = - let data = String.copy data in - for i = 0 to String.length data - 1 + let data = Bytes.copy data in + for i = 0 to Bytes.length data - 1 do - if data.[i] = '\000' then - data.[i] <- ' ' + if Bytes.get data i = '\000' then + Bytes.set data i ' ' done; - String.escaped data + String.escaped (Bytes.to_string data) let activate_access_log = ref true let access_log_destination = ref (File (Paths.xen_log_dir ^ "/xenstored-access.log")) @@ -291,7 +291,7 @@ let access_logging ~con ~tid ?(data="") ~level access_type = let date = string_of_date() in let tid = string_of_tid ~con tid in let access_type = string_of_access_type access_type in - let data = sanitize_data data in + let data = sanitize_data (Bytes.of_string data) in let prefix = prefix !access_log_destination date in let msg = Printf.sprintf "%s %s %s %s" prefix tid access_type data in logger.write ~level msg) diff --git a/tools/ocaml/xenstored/stdext.ml b/tools/ocaml/xenstored/stdext.ml index b8a8fd00e1..d05155c97e 100644 --- a/tools/ocaml/xenstored/stdext.ml +++ b/tools/ocaml/xenstored/stdext.ml @@ -122,7 +122,7 @@ let pidfile_write filename = let pid = Unix.getpid () in let buf = string_of_int pid ^ "\n" in let len = String.length buf in - if Unix.write fd buf 0 len <> len + if Unix.write fd (Bytes.of_string buf) 0 len <> len then failwith "pidfile_write failed"; ) (fun () -> Unix.close fd) diff --git a/tools/ocaml/xenstored/utils.ml b/tools/ocaml/xenstored/utils.ml index e89c1aff04..c96def7bee 100644 --- a/tools/ocaml/xenstored/utils.ml +++ b/tools/ocaml/xenstored/utils.ml @@ -45,23 +45,23 @@ let get_hierarchy path = let hexify s = let hexseq_of_char c = sprintf "%02x" (Char.code c) in - let hs = String.create (String.length s * 2) in + let hs = Bytes.create (String.length s * 2) in for i = 0 to String.length s - 1 do let seq = hexseq_of_char s.[i] in - hs.[i * 2] <- seq.[0]; - hs.[i * 2 + 1] <- seq.[1]; + Bytes.set hs (i * 2) seq.[0]; + Bytes.set hs (i * 2 + 1) seq.[1]; done; - hs + Bytes.to_string hs let unhexify hs = let char_of_hexseq seq0 seq1 = Char.chr (int_of_string (sprintf "0x%c%c" seq0 seq1)) in - let s = String.create (String.length hs / 2) in - for i = 0 to String.length s - 1 + let s = Bytes.create (String.length hs / 2) in + for i = 0 to Bytes.length s - 1 do - s.[i] <- char_of_hexseq hs.[i * 2] hs.[i * 2 + 1] + Bytes.set s i (char_of_hexseq hs.[i * 2] hs.[i * 2 + 1]) done; - s + Bytes.to_string s let trim_path path = try @@ -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) -- 2.14.3 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] make xen ocaml safe-strings compliant 2018-01-30 22:55 [PATCH 1/2] make xen ocaml safe-strings compliant Michael Young @ 2018-02-06 16:49 ` Wei Liu 2018-02-06 21:56 ` Michael Young 2018-02-08 17:49 ` Dario Faggioli 1 sibling, 1 reply; 18+ messages in thread From: Wei Liu @ 2018-02-06 16:49 UTC (permalink / raw) To: Michael Young Cc: Wei Liu, John Thomson, Marcello Seri, Christian Lindig, David Scott, xen-devel On Tue, Jan 30, 2018 at 10:55:47PM +0000, Michael Young wrote: > Xen built with ocaml 4.06 gives errors such as > Error: This expression has type bytes but an expression was > expected of type string > as Byte and safe-strings which were introduced in 4.02 are the > default in 4.06. > This patch which is partly by Richard W.M. Jones of Red Hat > from https://bugzilla.redhat.com/show_bug.cgi?id=1526703 > fixes these issues. > > Signed-off-by: Michael Young <m.a.young@durham.ac.uk> > Reviewed-by: Christian Lindig <christian.lindig@citrix.com> Strangely this doesn't apply to staging. And after examining the downloaded patch I'm not sure if my mail client is acting up. Do you have a git branch that I can pull from? Wei. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] make xen ocaml safe-strings compliant 2018-02-06 16:49 ` Wei Liu @ 2018-02-06 21:56 ` Michael Young 2018-02-07 10:31 ` Wei Liu 0 siblings, 1 reply; 18+ messages in thread From: Michael Young @ 2018-02-06 21:56 UTC (permalink / raw) To: Wei Liu Cc: Marcello Seri, xen-devel, Christian Lindig, John Thomson, David Scott [-- Attachment #1: Type: text/plain, Size: 990 bytes --] On Tue, 6 Feb 2018, Wei Liu wrote: > On Tue, Jan 30, 2018 at 10:55:47PM +0000, Michael Young wrote: >> Xen built with ocaml 4.06 gives errors such as >> Error: This expression has type bytes but an expression was >> expected of type string >> as Byte and safe-strings which were introduced in 4.02 are the >> default in 4.06. >> This patch which is partly by Richard W.M. Jones of Red Hat >> from https://bugzilla.redhat.com/show_bug.cgi?id=1526703 >> fixes these issues. >> >> Signed-off-by: Michael Young <m.a.young@durham.ac.uk> >> Reviewed-by: Christian Lindig <christian.lindig@citrix.com> > > Strangely this doesn't apply to staging. And after examining the > downloaded patch I'm not sure if my mail client is acting up. Do you > have a git branch that I can pull from? The patch needed to be reduced as one of the sections being patched was removed by a recent patch. I am attaching the revised patch as a file in case there was also an email formatting issue. Michael Young [-- Attachment #2: Type: text/plain, Size: 6150 bytes --] From 247cea9b587baafaf80bcc5b44bc68defb4efa26 Mon Sep 17 00:00:00 2001 From: Michael Young <m.a.young@durham.ac.uk> Date: Tue, 6 Feb 2018 21:27:23 +0000 Subject: [PATCH 1/2 v2] make xen ocaml safe-strings compliant Xen built with ocaml 4.06 gives errors such as Error: This expression has type bytes but an expression was expected of type string as Byte and safe-strings which were introduced in 4.02 are the default in 4.06. This patch which is mostly by Richard W.M. Jones of Red Hat from https://bugzilla.redhat.com/show_bug.cgi?id=1526703 fixes these issues. v2: drop tools/ocaml/libs/xc/xenctrl.ml from the patch as the affected code was removed by commit d933f1a53c06002351c1e36d40615e40bd4bf6af tools/ocaml: Drop coredump infrastructure Signed-off-by: Michael Young <m.a.young@durham.ac.uk> Reviewed-by: Christian Lindig <christian.lindig@citrix.com> --- tools/ocaml/libs/xb/xb.ml | 6 +++--- tools/ocaml/xenstored/logging.ml | 22 +++++++++++----------- tools/ocaml/xenstored/stdext.ml | 2 +- tools/ocaml/xenstored/utils.ml | 18 +++++++++--------- 4 files changed, 24 insertions(+), 24 deletions(-) diff --git a/tools/ocaml/libs/xb/xb.ml b/tools/ocaml/libs/xb/xb.ml index 50944b5fd6..aa2cf98223 100644 --- a/tools/ocaml/libs/xb/xb.ml +++ b/tools/ocaml/libs/xb/xb.ml @@ -84,7 +84,7 @@ let read_mmap back con s len = let read con s len = match con.backend with - | Fd backfd -> read_fd backfd con s len + | Fd backfd -> read_fd backfd con (Bytes.of_string s) len | Xenmmap backmmap -> read_mmap backmmap con s len let write_fd back con s len = @@ -98,7 +98,7 @@ let write_mmap back con s len = let write con s len = match con.backend with - | Fd backfd -> write_fd backfd con s len + | Fd backfd -> write_fd backfd con (Bytes.of_string s) len | Xenmmap backmmap -> write_mmap backmmap con s len (* NB: can throw Reconnect *) @@ -147,7 +147,7 @@ let input con = | NoHdr (i, buf) -> (* we complete the partial header *) if sz > 0 then - String.blit s 0 buf (Partial.header_size () - i) sz; + String.blit s 0 (Bytes.of_string buf) (Partial.header_size () - i) sz; con.partial_in <- if sz = i then HaveHdr (Partial.of_string buf) else NoHdr (i - sz, buf) ); diff --git a/tools/ocaml/xenstored/logging.ml b/tools/ocaml/xenstored/logging.ml index 0c0d03d0c4..d24abf8a3a 100644 --- a/tools/ocaml/xenstored/logging.ml +++ b/tools/ocaml/xenstored/logging.ml @@ -60,11 +60,11 @@ type logger = let truncate_line nb_chars line = if String.length line > nb_chars - 1 then let len = max (nb_chars - 1) 2 in - let dst_line = String.create len in - String.blit line 0 dst_line 0 (len - 2); - dst_line.[len-2] <- '.'; - dst_line.[len-1] <- '.'; - dst_line + let dst_line = Bytes.create len in + Bytes.blit_string line 0 dst_line 0 (len - 2); + Bytes.set dst_line (len-2) '.'; + Bytes.set dst_line (len-1) '.'; + Bytes.to_string dst_line else line let log_rotate ref_ch log_file log_nb_files = @@ -252,13 +252,13 @@ let string_of_access_type = function *) let sanitize_data data = - let data = String.copy data in - for i = 0 to String.length data - 1 + let data = Bytes.copy data in + for i = 0 to Bytes.length data - 1 do - if data.[i] = '\000' then - data.[i] <- ' ' + if Bytes.get data i = '\000' then + Bytes.set data i ' ' done; - String.escaped data + String.escaped (Bytes.to_string data) let activate_access_log = ref true let access_log_destination = ref (File (Paths.xen_log_dir ^ "/xenstored-access.log")) @@ -291,7 +291,7 @@ let access_logging ~con ~tid ?(data="") ~level access_type = let date = string_of_date() in let tid = string_of_tid ~con tid in let access_type = string_of_access_type access_type in - let data = sanitize_data data in + let data = sanitize_data (Bytes.of_string data) in let prefix = prefix !access_log_destination date in let msg = Printf.sprintf "%s %s %s %s" prefix tid access_type data in logger.write ~level msg) diff --git a/tools/ocaml/xenstored/stdext.ml b/tools/ocaml/xenstored/stdext.ml index b8a8fd00e1..d05155c97e 100644 --- a/tools/ocaml/xenstored/stdext.ml +++ b/tools/ocaml/xenstored/stdext.ml @@ -122,7 +122,7 @@ let pidfile_write filename = let pid = Unix.getpid () in let buf = string_of_int pid ^ "\n" in let len = String.length buf in - if Unix.write fd buf 0 len <> len + if Unix.write fd (Bytes.of_string buf) 0 len <> len then failwith "pidfile_write failed"; ) (fun () -> Unix.close fd) diff --git a/tools/ocaml/xenstored/utils.ml b/tools/ocaml/xenstored/utils.ml index e89c1aff04..c96def7bee 100644 --- a/tools/ocaml/xenstored/utils.ml +++ b/tools/ocaml/xenstored/utils.ml @@ -45,23 +45,23 @@ let get_hierarchy path = let hexify s = let hexseq_of_char c = sprintf "%02x" (Char.code c) in - let hs = String.create (String.length s * 2) in + let hs = Bytes.create (String.length s * 2) in for i = 0 to String.length s - 1 do let seq = hexseq_of_char s.[i] in - hs.[i * 2] <- seq.[0]; - hs.[i * 2 + 1] <- seq.[1]; + Bytes.set hs (i * 2) seq.[0]; + Bytes.set hs (i * 2 + 1) seq.[1]; done; - hs + Bytes.to_string hs let unhexify hs = let char_of_hexseq seq0 seq1 = Char.chr (int_of_string (sprintf "0x%c%c" seq0 seq1)) in - let s = String.create (String.length hs / 2) in - for i = 0 to String.length s - 1 + let s = Bytes.create (String.length hs / 2) in + for i = 0 to Bytes.length s - 1 do - s.[i] <- char_of_hexseq hs.[i * 2] hs.[i * 2 + 1] + Bytes.set s i (char_of_hexseq hs.[i * 2] hs.[i * 2 + 1]) done; - s + Bytes.to_string s let trim_path path = try @@ -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) -- 2.14.3 [-- Attachment #3: Type: text/plain, Size: 157 bytes --] _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] make xen ocaml safe-strings compliant 2018-02-06 21:56 ` Michael Young @ 2018-02-07 10:31 ` Wei Liu 0 siblings, 0 replies; 18+ messages in thread From: Wei Liu @ 2018-02-07 10:31 UTC (permalink / raw) To: Michael Young Cc: Wei Liu, John Thomson, Marcello Seri, Christian Lindig, David Scott, xen-devel On Tue, Feb 06, 2018 at 09:56:14PM +0000, Michael Young wrote: > On Tue, 6 Feb 2018, Wei Liu wrote: > > > On Tue, Jan 30, 2018 at 10:55:47PM +0000, Michael Young wrote: > > > Xen built with ocaml 4.06 gives errors such as > > > Error: This expression has type bytes but an expression was > > > expected of type string > > > as Byte and safe-strings which were introduced in 4.02 are the > > > default in 4.06. > > > This patch which is partly by Richard W.M. Jones of Red Hat > > > from https://bugzilla.redhat.com/show_bug.cgi?id=1526703 > > > fixes these issues. > > > > > > Signed-off-by: Michael Young <m.a.young@durham.ac.uk> > > > Reviewed-by: Christian Lindig <christian.lindig@citrix.com> > > > > Strangely this doesn't apply to staging. And after examining the > > downloaded patch I'm not sure if my mail client is acting up. Do you > > have a git branch that I can pull from? > > The patch needed to be reduced as one of the sections being patched was > removed by a recent patch. I am attaching the revised patch as a file in > case there was also an email formatting issue. > Thanks, I will try this today. Wei. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] make xen ocaml safe-strings compliant 2018-01-30 22:55 [PATCH 1/2] make xen ocaml safe-strings compliant Michael Young 2018-02-06 16:49 ` Wei Liu @ 2018-02-08 17:49 ` Dario Faggioli 2018-02-08 18:03 ` Wei Liu 1 sibling, 1 reply; 18+ messages in thread From: Dario Faggioli @ 2018-02-08 17:49 UTC (permalink / raw) To: Michael Young, xen-devel Cc: Marcello Seri, Wei Liu, Christian Lindig, John Thomson, David Scott [-- Attachment #1.1: Type: text/plain, Size: 1576 bytes --] On Tue, 2018-01-30 at 22:55 +0000, Michael Young wrote: > Xen built with ocaml 4.06 gives errors such as > Error: This expression has type bytes but an expression was > expected of type string > as Byte and safe-strings which were introduced in 4.02 are the > default in 4.06. > This patch which is partly by Richard W.M. Jones of Red Hat > from https://bugzilla.redhat.com/show_bug.cgi?id=1526703 > fixes these issues. > > Signed-off-by: Michael Young <m.a.young@durham.ac.uk> > Reviewed-by: Christian Lindig <christian.lindig@citrix.com> > So, with this patch, oxenstord does not start for me. Systemd says this (sorry, it's not the full output.. I don't have it right now, but can produce it): systemctl status xenstored ... Active: failed (Result: protocol) since Thu 2018-02-08 17:47:56 CET; 12min ago ... Feb 08 17:47:56 Zhaman systemd[1]: xenstored.service: Failed with result 'protocol'. Just running oxenstored from command line seems to exits with 0, but there's not an oxenstored process running. Getting rid of what is now commit "make xen ocaml safe-strings compliant" (df1e4c6e7f8892e950433ff33c215df0cd7b30f7), things work again for me. Regards, Dario PS. There has been a v2 of this patch, I think, but I don't have in my emails any longer, so I'm replying to this one. -- <<This happens because I choose it to happen!>> (Raistlin Majere) ----------------------------------------------------------------- Dario Faggioli, Ph.D, http://about.me/dario.faggioli Software Engineer @ SUSE https://www.suse.com/ [-- Attachment #1.2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 833 bytes --] [-- Attachment #2: Type: text/plain, Size: 157 bytes --] _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] make xen ocaml safe-strings compliant 2018-02-08 17:49 ` Dario Faggioli @ 2018-02-08 18:03 ` Wei Liu 2018-02-08 18:24 ` Wei Liu 0 siblings, 1 reply; 18+ messages in thread From: Wei Liu @ 2018-02-08 18:03 UTC (permalink / raw) To: Dario Faggioli Cc: Wei Liu, David Scott, John Thomson, Marcello Seri, Christian Lindig, Michael Young, xen-devel On Thu, Feb 08, 2018 at 06:49:58PM +0100, Dario Faggioli wrote: > On Tue, 2018-01-30 at 22:55 +0000, Michael Young wrote: > > Xen built with ocaml 4.06 gives errors such as > > Error: This expression has type bytes but an expression was > > expected of type string > > as Byte and safe-strings which were introduced in 4.02 are the > > default in 4.06. > > This patch which is partly by Richard W.M. Jones of Red Hat > > from https://bugzilla.redhat.com/show_bug.cgi?id=1526703 > > fixes these issues. > > > > Signed-off-by: Michael Young <m.a.young@durham.ac.uk> > > Reviewed-by: Christian Lindig <christian.lindig@citrix.com> > > > So, with this patch, oxenstord does not start for me. > > Systemd says this (sorry, it's not the full output.. I don't have it > right now, but can produce it): > > systemctl status xenstored > ... > Active: failed (Result: protocol) since Thu 2018-02-08 17:47:56 CET; 12min ago > ... > Feb 08 17:47:56 Zhaman systemd[1]: xenstored.service: Failed with result 'protocol'. > > Just running oxenstored from command line seems to exits with 0, but > there's not an oxenstored process running. > > Getting rid of what is now commit "make xen ocaml safe-strings > compliant" (df1e4c6e7f8892e950433ff33c215df0cd7b30f7), things work > again for me. > OK. I will revert the relevant commits in staging. I'm sure this will block osstest flights. Wei. > Regards, > Dario > > PS. There has been a v2 of this patch, I think, but I don't have in my > emails any longer, so I'm replying to this one. > > -- > <<This happens because I choose it to happen!>> (Raistlin Majere) > ----------------------------------------------------------------- > Dario Faggioli, Ph.D, http://about.me/dario.faggioli > Software Engineer @ SUSE https://www.suse.com/ _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] make xen ocaml safe-strings compliant 2018-02-08 18:03 ` Wei Liu @ 2018-02-08 18:24 ` Wei Liu 2018-02-09 9:20 ` Christian Lindig 0 siblings, 1 reply; 18+ messages in thread From: Wei Liu @ 2018-02-08 18:24 UTC (permalink / raw) To: Dario Faggioli Cc: Wei Liu, David Scott, John Thomson, Marcello Seri, Christian Lindig, Michael Young, xen-devel On Thu, Feb 08, 2018 at 06:03:48PM +0000, Wei Liu wrote: > On Thu, Feb 08, 2018 at 06:49:58PM +0100, Dario Faggioli wrote: > > On Tue, 2018-01-30 at 22:55 +0000, Michael Young wrote: > > > Xen built with ocaml 4.06 gives errors such as > > > Error: This expression has type bytes but an expression was > > > expected of type string > > > as Byte and safe-strings which were introduced in 4.02 are the > > > default in 4.06. > > > This patch which is partly by Richard W.M. Jones of Red Hat > > > from https://bugzilla.redhat.com/show_bug.cgi?id=1526703 > > > fixes these issues. > > > > > > Signed-off-by: Michael Young <m.a.young@durham.ac.uk> > > > Reviewed-by: Christian Lindig <christian.lindig@citrix.com> > > > > > So, with this patch, oxenstord does not start for me. > > > > Systemd says this (sorry, it's not the full output.. I don't have it > > right now, but can produce it): > > > > systemctl status xenstored > > ... > > Active: failed (Result: protocol) since Thu 2018-02-08 17:47:56 CET; 12min ago > > ... > > Feb 08 17:47:56 Zhaman systemd[1]: xenstored.service: Failed with result 'protocol'. > > > > Just running oxenstored from command line seems to exits with 0, but > > there's not an oxenstored process running. > > > > Getting rid of what is now commit "make xen ocaml safe-strings > > compliant" (df1e4c6e7f8892e950433ff33c215df0cd7b30f7), things work > > again for me. > > > > OK. I will revert the relevant commits in staging. I'm sure this will > block osstest flights. > Correction: osstest currently still runs Debian jessie, which has ocaml 4.01, which means the bump to 4.02 in staging is likely cause oxenstored to be disabled. New flights could still pass but that's due to oxenstored not getting tested. It is convoluted, I know. :-/ I need to at least revert the safe-string patches in staging. As for the version bump, I'm not so sure. On one hand it is useless in its own and leaving the bump in tree actually stops the testing of oxenstored, on the other hand it is a must-have for safe-string patch, assuming we will have a proper safe-string fix soon-ish we can leave them in staging. Christian, do you have any idea when you can look into fixing the safe-string patch? And osstest should really be upgraded to strech (which has 4.02). Wei. > Wei. > > > Regards, > > Dario > > > > PS. There has been a v2 of this patch, I think, but I don't have in my > > emails any longer, so I'm replying to this one. > > > > -- > > <<This happens because I choose it to happen!>> (Raistlin Majere) > > ----------------------------------------------------------------- > > Dario Faggioli, Ph.D, http://about.me/dario.faggioli > > Software Engineer @ SUSE https://www.suse.com/ > > _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] make xen ocaml safe-strings compliant 2018-02-08 18:24 ` Wei Liu @ 2018-02-09 9:20 ` Christian Lindig 2018-02-09 10:06 ` Dario Faggioli ` (2 more replies) 0 siblings, 3 replies; 18+ messages in thread From: Christian Lindig @ 2018-02-09 9:20 UTC (permalink / raw) To: Wei Liu Cc: David Scott, John Thomson, Dario Faggioli, Marcello Seri, Michael Young, Xen-devel > On 8. Feb 2018, at 18:24, Wei Liu <wei.liu2@citrix.com> wrote: > > Christian, do you have any idea when you can look into fixing the > safe-string patch? Sorry, I can’t make a promise because of my other obligations. I do wonder, though: this patch did not come out of nowhere but supposedly was working - what is different here? In any case, I will reproduce the problem and take a look. The patch was doing the right thing for the future but there is no harm not having it right away. The entire XenServer toolstack is facing the same problem of moving to immutable strings and so far we have done it where it was easy and moved to use upstream libraries that use immutable string. In our own code, this is still a massive task. — Christian _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] make xen ocaml safe-strings compliant 2018-02-09 9:20 ` Christian Lindig @ 2018-02-09 10:06 ` Dario Faggioli 2018-02-12 14:55 ` Wei Liu 2018-02-13 0:35 ` Michael Young 2 siblings, 0 replies; 18+ messages in thread From: Dario Faggioli @ 2018-02-09 10:06 UTC (permalink / raw) To: Christian Lindig, Wei Liu Cc: Marcello Seri, Xen-devel, David Scott, John Thomson, Michael Young [-- Attachment #1.1: Type: text/plain, Size: 1200 bytes --] On Fri, 2018-02-09 at 09:20 +0000, Christian Lindig wrote: > > On 8. Feb 2018, at 18:24, Wei Liu <wei.liu2@citrix.com> wrote: > > > > Christian, do you have any idea when you can look into fixing the > > safe-string patch? > > Sorry, I can’t make a promise because of my other obligations. I do > wonder, though: this patch did not come out of nowhere but supposedly > was working - what is different here? > Well, the only thing that I know about OCAML is that it is, for me as an Italian, a word a little bit difficult to pronounce. :-D But I'm happy to try to give some more details on what fails and how, if you direct me. :-) The testbox where this happens is a Debian unstable which has ocaml 4.05.0-10. > In any case, I will reproduce the problem and take a look. > Andrew said on IRC that he's seen something similar happening on some of yours XenRT runs... but really, I'm happy to provide more info. Regards, Dario -- <<This happens because I choose it to happen!>> (Raistlin Majere) ----------------------------------------------------------------- Dario Faggioli, Ph.D, http://about.me/dario.faggioli Software Engineer @ SUSE https://www.suse.com/ [-- Attachment #1.2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 833 bytes --] [-- Attachment #2: Type: text/plain, Size: 157 bytes --] _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] make xen ocaml safe-strings compliant 2018-02-09 9:20 ` Christian Lindig 2018-02-09 10:06 ` Dario Faggioli @ 2018-02-12 14:55 ` Wei Liu 2018-03-09 22:57 ` Michael Young 2018-02-13 0:35 ` Michael Young 2 siblings, 1 reply; 18+ messages in thread From: Wei Liu @ 2018-02-12 14:55 UTC (permalink / raw) To: Christian Lindig Cc: Wei Liu, David Scott, John Thomson, Dario Faggioli, Marcello Seri, Michael Young, Xen-devel On Fri, Feb 09, 2018 at 09:20:33AM +0000, Christian Lindig wrote: > > > > On 8. Feb 2018, at 18:24, Wei Liu <wei.liu2@citrix.com> wrote: > > > > Christian, do you have any idea when you can look into fixing the > > safe-string patch? > > Sorry, I can’t make a promise because of my other obligations. I do wonder, though: this patch did not come out of nowhere but supposedly was working - what is different here? > No worries. I have reverted some patches in xen.git to get things going again. Wei. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] make xen ocaml safe-strings compliant 2018-02-12 14:55 ` Wei Liu @ 2018-03-09 22:57 ` Michael Young 2018-03-09 23:47 ` Christian Lindig 2018-03-12 11:29 ` Christian Lindig 0 siblings, 2 replies; 18+ messages in thread From: Michael Young @ 2018-03-09 22:57 UTC (permalink / raw) To: Wei Liu Cc: John Thomson, Dario Faggioli, Marcello Seri, Christian Lindig, David Scott, Xen-devel [-- Attachment #1: Type: text/plain, Size: 689 bytes --] On Mon, 12 Feb 2018, Wei Liu wrote: > On Fri, Feb 09, 2018 at 09:20:33AM +0000, Christian Lindig wrote: >> >> >>> On 8. Feb 2018, at 18:24, Wei Liu <wei.liu2@citrix.com> wrote: >>> >>> Christian, do you have any idea when you can look into fixing the >>> safe-string patch? >> >> Sorry, I can’t make a promise because of my other obligations. I do wonder, though: this patch did not come out of nowhere but supposedly was working - what is different here? >> > > No worries. I have reverted some patches in xen.git to get things going > again. I have had a go at fixing the patch and my revised attempt is attached. I suspect it could be tidied up, but it works for me. Michael Young [-- Attachment #2: Type: text/plain, Size: 8719 bytes --] From 550ffe177842e3fd9f38c78e07072fa7c7b591a5 Mon Sep 17 00:00:00 2001 From: Michael Young <m.a.young@durham.ac.uk> Date: Fri, 9 Mar 2018 22:31:41 +0000 Subject: [PATCH v3] make xen ocaml safe-strings compliant Xen built with ocaml 4.06 gives errors such as Error: This expression has type bytes but an expression was expected of type string as Byte and safe-strings which were introduced in 4.02 are the default in 4.06. This patch which is partly by Richard W.M. Jones of Red Hat from https://bugzilla.redhat.com/show_bug.cgi?id=1526703 fixes these issues. v3: rework patches for xb.ml and /utils.ml to fix broken code relating to Unix.read. Update xb.mli to match changes in xb.ml. Signed-off-by: Michael Young <m.a.young@durham.ac.uk> --- tools/ocaml/libs/xb/xb.ml | 18 ++++++++++-------- tools/ocaml/libs/xb/xb.mli | 10 +++++----- tools/ocaml/xenstored/logging.ml | 22 +++++++++++----------- tools/ocaml/xenstored/stdext.ml | 2 +- tools/ocaml/xenstored/utils.ml | 20 ++++++++++---------- 5 files changed, 37 insertions(+), 35 deletions(-) diff --git a/tools/ocaml/libs/xb/xb.ml b/tools/ocaml/libs/xb/xb.ml index 50944b5fd6..42ae8d2bd8 100644 --- a/tools/ocaml/libs/xb/xb.ml +++ b/tools/ocaml/libs/xb/xb.ml @@ -40,7 +40,7 @@ type backend_fd = type backend = Fd of backend_fd | Xenmmap of backend_mmap -type partial_buf = HaveHdr of Partial.pkt | NoHdr of int * string +type partial_buf = HaveHdr of Partial.pkt | NoHdr of int * bytes type t = { @@ -52,7 +52,7 @@ type t = } let init_partial_in () = NoHdr - (Partial.header_size (), String.make (Partial.header_size()) '\000') + (Partial.header_size (), Bytes.make (Partial.header_size()) '\000') let reconnect t = match t.backend with | Fd _ -> @@ -76,7 +76,9 @@ let read_fd back con s len = rd 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 (); @@ -98,7 +100,7 @@ let write_mmap back con s len = let write con s len = match con.backend with - | Fd backfd -> write_fd backfd con s len + | Fd backfd -> write_fd backfd con (Bytes.of_string s) len | Xenmmap backmmap -> write_mmap backmmap con s len (* NB: can throw Reconnect *) @@ -129,7 +131,7 @@ let input con = | NoHdr (i, buf) -> i in (* try to get more data from input stream *) - let s = String.make to_read '\000' in + let s = Bytes.make to_read '\000' in let sz = if to_read > 0 then read con s to_read else 0 in ( @@ -137,7 +139,7 @@ let input con = | HaveHdr partial_pkt -> (* we complete the data *) if sz > 0 then - Partial.append partial_pkt s sz; + Partial.append partial_pkt (Bytes.to_string s) sz; if Partial.to_complete partial_pkt = 0 then ( let pkt = Packet.of_partialpkt partial_pkt in con.partial_in <- init_partial_in (); @@ -147,9 +149,9 @@ let input con = | NoHdr (i, buf) -> (* we complete the partial header *) if sz > 0 then - String.blit s 0 buf (Partial.header_size () - i) sz; + Bytes.blit s 0 buf (Partial.header_size () - i) sz; con.partial_in <- if sz = i then - HaveHdr (Partial.of_string buf) else NoHdr (i - sz, buf) + HaveHdr (Partial.of_string (Bytes.to_string buf)) else NoHdr (i - sz, buf) ); !newpacket diff --git a/tools/ocaml/libs/xb/xb.mli b/tools/ocaml/libs/xb/xb.mli index b4d705201f..d566011fc7 100644 --- a/tools/ocaml/libs/xb/xb.mli +++ b/tools/ocaml/libs/xb/xb.mli @@ -65,7 +65,7 @@ type backend_mmap = { } type backend_fd = { fd : Unix.file_descr; } type backend = Fd of backend_fd | Xenmmap of backend_mmap -type partial_buf = HaveHdr of Partial.pkt | NoHdr of int * string +type partial_buf = HaveHdr of Partial.pkt | NoHdr of int * bytes type t = { backend : backend; pkt_in : Packet.t Queue.t; @@ -76,10 +76,10 @@ type t = { val init_partial_in : unit -> partial_buf val reconnect : t -> unit val queue : t -> Packet.t -> unit -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 val write_mmap : backend_mmap -> 'a -> string -> int -> int val write : t -> string -> int -> int val output : t -> bool diff --git a/tools/ocaml/xenstored/logging.ml b/tools/ocaml/xenstored/logging.ml index 0c0d03d0c4..d24abf8a3a 100644 --- a/tools/ocaml/xenstored/logging.ml +++ b/tools/ocaml/xenstored/logging.ml @@ -60,11 +60,11 @@ type logger = let truncate_line nb_chars line = if String.length line > nb_chars - 1 then let len = max (nb_chars - 1) 2 in - let dst_line = String.create len in - String.blit line 0 dst_line 0 (len - 2); - dst_line.[len-2] <- '.'; - dst_line.[len-1] <- '.'; - dst_line + let dst_line = Bytes.create len in + Bytes.blit_string line 0 dst_line 0 (len - 2); + Bytes.set dst_line (len-2) '.'; + Bytes.set dst_line (len-1) '.'; + Bytes.to_string dst_line else line let log_rotate ref_ch log_file log_nb_files = @@ -252,13 +252,13 @@ let string_of_access_type = function *) let sanitize_data data = - let data = String.copy data in - for i = 0 to String.length data - 1 + let data = Bytes.copy data in + for i = 0 to Bytes.length data - 1 do - if data.[i] = '\000' then - data.[i] <- ' ' + if Bytes.get data i = '\000' then + Bytes.set data i ' ' done; - String.escaped data + String.escaped (Bytes.to_string data) let activate_access_log = ref true let access_log_destination = ref (File (Paths.xen_log_dir ^ "/xenstored-access.log")) @@ -291,7 +291,7 @@ let access_logging ~con ~tid ?(data="") ~level access_type = let date = string_of_date() in let tid = string_of_tid ~con tid in let access_type = string_of_access_type access_type in - let data = sanitize_data data in + let data = sanitize_data (Bytes.of_string data) in let prefix = prefix !access_log_destination date in let msg = Printf.sprintf "%s %s %s %s" prefix tid access_type data in logger.write ~level msg) diff --git a/tools/ocaml/xenstored/stdext.ml b/tools/ocaml/xenstored/stdext.ml index b8a8fd00e1..d05155c97e 100644 --- a/tools/ocaml/xenstored/stdext.ml +++ b/tools/ocaml/xenstored/stdext.ml @@ -122,7 +122,7 @@ let pidfile_write filename = let pid = Unix.getpid () in let buf = string_of_int pid ^ "\n" in let len = String.length buf in - if Unix.write fd buf 0 len <> len + if Unix.write fd (Bytes.of_string buf) 0 len <> len then failwith "pidfile_write failed"; ) (fun () -> Unix.close fd) diff --git a/tools/ocaml/xenstored/utils.ml b/tools/ocaml/xenstored/utils.ml index e89c1aff04..4fc542dd51 100644 --- a/tools/ocaml/xenstored/utils.ml +++ b/tools/ocaml/xenstored/utils.ml @@ -45,23 +45,23 @@ let get_hierarchy path = let hexify s = let hexseq_of_char c = sprintf "%02x" (Char.code c) in - let hs = String.create (String.length s * 2) in + let hs = Bytes.create (String.length s * 2) in for i = 0 to String.length s - 1 do let seq = hexseq_of_char s.[i] in - hs.[i * 2] <- seq.[0]; - hs.[i * 2 + 1] <- seq.[1]; + Bytes.set hs (i * 2) seq.[0]; + Bytes.set hs (i * 2 + 1) seq.[1]; done; - hs + Bytes.to_string hs let unhexify hs = let char_of_hexseq seq0 seq1 = Char.chr (int_of_string (sprintf "0x%c%c" seq0 seq1)) in - let s = String.create (String.length hs / 2) in - for i = 0 to String.length s - 1 + let s = Bytes.create (String.length hs / 2) in + for i = 0 to Bytes.length s - 1 do - s.[i] <- char_of_hexseq hs.[i * 2] hs.[i * 2 + 1] + Bytes.set s i (char_of_hexseq hs.[i * 2] hs.[i * 2 + 1]) done; - s + Bytes.to_string s let trim_path path = try @@ -84,10 +84,10 @@ 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 buf = Bytes.make 20 (char_of_int 0) in let sz = Unix.read fd buf 0 20 in Unix.close fd; - int_of_string (String.sub buf 0 sz) + int_of_string (Bytes.to_string (Bytes.sub buf 0 sz)) let path_complete path connection_path = if String.get path 0 <> '/' then -- 2.14.3 [-- Attachment #3: Type: text/plain, Size: 157 bytes --] _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] make xen ocaml safe-strings compliant 2018-03-09 22:57 ` Michael Young @ 2018-03-09 23:47 ` Christian Lindig 2018-03-10 0:36 ` Michael Young 2018-03-12 11:29 ` Christian Lindig 1 sibling, 1 reply; 18+ messages in thread From: Christian Lindig @ 2018-03-09 23:47 UTC (permalink / raw) To: Michael Young Cc: Wei Liu, John Thomson, Dario Faggioli, Marcello Seri, David Scott, Xen-devel > On 9. Mar 2018, at 22:57, Michael Young <m.a.young@durham.ac.uk> wrote: > > I have had a go at fixing the patch and my revised attempt is attached. I suspect it could be tidied up, but it works for me. Thank you for giving this another go. What is the key difference to the previous patch which compiled but lead to lock up at run time? I briefly tried your patch and can confirm that it compiles and looks clean except for two trailing spaces. — Christian Acked-by: Christian Lindig <christian.lindig@citrix.com> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] make xen ocaml safe-strings compliant 2018-03-09 23:47 ` Christian Lindig @ 2018-03-10 0:36 ` Michael Young 0 siblings, 0 replies; 18+ messages in thread From: Michael Young @ 2018-03-10 0:36 UTC (permalink / raw) To: Christian Lindig Cc: Wei Liu, John Thomson, Dario Faggioli, Marcello Seri, David Scott, Xen-devel [-- Attachment #1: Type: text/plain, Size: 2108 bytes --] On Fri, 9 Mar 2018, Christian Lindig wrote: > > >> On 9. Mar 2018, at 22:57, Michael Young <m.a.young@durham.ac.uk> wrote: >> >> I have had a go at fixing the patch and my revised attempt is attached. I suspect it could be tidied up, but it works for me. > > Thank you for giving this another go. What is the key difference to the previous patch which compiled but lead to lock up at run time? I briefly tried your patch and can confirm that it compiles and looks clean except for two trailing spaces. > > — Christian > > Acked-by: Christian Lindig <christian.lindig@citrix.com> 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. The net result is that information being read by oxenstored from the hypervisor is corrupted or lost. The same basic problem also occurred in a couple of places in the old patch of tools/ocaml/libs/xb/xb.ml. My fix for this is to switch to Bytes at an earlier stage, so for example the corresponding section in the new patch becomes @@ -84,10 +84,10 @@ 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 buf = Bytes.make 20 (char_of_int 0) in let sz = Unix.read fd buf 0 20 in Unix.close fd; - int_of_string (String.sub buf 0 sz) + int_of_string (Bytes.to_string (Bytes.sub buf 0 sz)) let path_complete path connection_path = if String.get path 0 <> '/' then Michael Young [-- Attachment #2: Type: text/plain, Size: 157 bytes --] _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] make xen ocaml safe-strings compliant 2018-03-09 22:57 ` Michael Young 2018-03-09 23:47 ` Christian Lindig @ 2018-03-12 11:29 ` Christian Lindig 2018-03-12 19:35 ` Michael Young 1 sibling, 1 reply; 18+ messages in thread From: Christian Lindig @ 2018-03-12 11:29 UTC (permalink / raw) To: Michael Young, Wei Liu Cc: Marcello Seri, Xen-devel, David Scott, John Thomson, Dario Faggioli > 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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] make xen ocaml safe-strings compliant 2018-03-12 11:29 ` Christian Lindig @ 2018-03-12 19:35 ` Michael Young 2018-03-13 9:29 ` Christian Lindig 0 siblings, 1 reply; 18+ messages in thread From: Michael Young @ 2018-03-12 19:35 UTC (permalink / raw) To: Christian Lindig Cc: Wei Liu, John Thomson, Dario Faggioli, Marcello Seri, David Scott, Xen-devel [-- Attachment #1: Type: text/plain, Size: 2385 bytes --] 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 #2: Type: text/plain, Size: 9432 bytes --] From da088e4eef2bbea4be262e12db4c36960ff5145a Mon Sep 17 00:00:00 2001 From: Michael Young <m.a.young@durham.ac.uk> Date: Mon, 12 Mar 2018 18:49:29 +0000 Subject: [PATCH v4] make xen ocaml safe-strings compliant Xen built with ocaml 4.06 gives errors such as Error: This expression has type bytes but an expression was expected of type string as Byte and safe-strings which were introduced in 4.02 are the default in 4.06. This patch which is partly by Richard W.M. Jones of Red Hat from https://bugzilla.redhat.com/show_bug.cgi?id=1526703 fixes these issues. v4: Where string s is now bytes, rename it to b. Signed-off-by: Michael Young <m.a.young@durham.ac.uk> --- tools/ocaml/libs/xb/xb.ml | 34 ++++++++++++++++++---------------- tools/ocaml/libs/xb/xb.mli | 10 +++++----- tools/ocaml/xenstored/logging.ml | 22 +++++++++++----------- tools/ocaml/xenstored/stdext.ml | 2 +- tools/ocaml/xenstored/utils.ml | 20 ++++++++++---------- 5 files changed, 45 insertions(+), 43 deletions(-) diff --git a/tools/ocaml/libs/xb/xb.ml b/tools/ocaml/libs/xb/xb.ml index 50944b5fd6..519842723b 100644 --- a/tools/ocaml/libs/xb/xb.ml +++ b/tools/ocaml/libs/xb/xb.ml @@ -40,7 +40,7 @@ type backend_fd = type backend = Fd of backend_fd | Xenmmap of backend_mmap -type partial_buf = HaveHdr of Partial.pkt | NoHdr of int * string +type partial_buf = HaveHdr of Partial.pkt | NoHdr of int * bytes type t = { @@ -52,7 +52,7 @@ type t = } let init_partial_in () = NoHdr - (Partial.header_size (), String.make (Partial.header_size()) '\000') + (Partial.header_size (), Bytes.make (Partial.header_size()) '\000') let reconnect t = match t.backend with | Fd _ -> @@ -69,26 +69,28 @@ let reconnect t = match t.backend with let queue con pkt = Queue.push pkt con.pkt_out -let read_fd back con s len = - let rd = Unix.read back.fd s 0 len in +let read_fd back con b len = + let rd = Unix.read back.fd b 0 len in if rd = 0 then raise End_of_file; rd -let read_mmap back con s len = +let read_mmap back con b len = + let s = String.make len (char_of_int 0) in let rd = Xs_ring.read back.mmap s len in + Bytes.blit_string s 0 b 0 rd; back.work_again <- (rd > 0); if rd > 0 then back.eventchn_notify (); rd -let read con s len = +let read con b len = match con.backend with - | Fd backfd -> read_fd backfd con s len - | Xenmmap backmmap -> read_mmap backmmap con s len + | Fd backfd -> read_fd backfd con b len + | Xenmmap backmmap -> read_mmap backmmap con b len -let write_fd back con s len = - Unix.write back.fd s 0 len +let write_fd back con b len = + Unix.write back.fd b 0 len let write_mmap back con s len = let ws = Xs_ring.write back.mmap s len in @@ -98,7 +100,7 @@ let write_mmap back con s len = let write con s len = match con.backend with - | Fd backfd -> write_fd backfd con s len + | Fd backfd -> write_fd backfd con (Bytes.of_string s) len | Xenmmap backmmap -> write_mmap backmmap con s len (* NB: can throw Reconnect *) @@ -129,15 +131,15 @@ let input con = | NoHdr (i, buf) -> i in (* try to get more data from input stream *) - let s = String.make to_read '\000' in - let sz = if to_read > 0 then read con s to_read else 0 in + let b = Bytes.make to_read '\000' in + let sz = if to_read > 0 then read con b to_read else 0 in ( match con.partial_in with | HaveHdr partial_pkt -> (* we complete the data *) if sz > 0 then - Partial.append partial_pkt s sz; + Partial.append partial_pkt (Bytes.to_string b) sz; if Partial.to_complete partial_pkt = 0 then ( let pkt = Packet.of_partialpkt partial_pkt in con.partial_in <- init_partial_in (); @@ -147,9 +149,9 @@ let input con = | NoHdr (i, buf) -> (* we complete the partial header *) if sz > 0 then - String.blit s 0 buf (Partial.header_size () - i) sz; + Bytes.blit b 0 buf (Partial.header_size () - i) sz; con.partial_in <- if sz = i then - HaveHdr (Partial.of_string buf) else NoHdr (i - sz, buf) + HaveHdr (Partial.of_string (Bytes.to_string buf)) else NoHdr (i - sz, buf) ); !newpacket diff --git a/tools/ocaml/libs/xb/xb.mli b/tools/ocaml/libs/xb/xb.mli index b4d705201f..d566011fc7 100644 --- a/tools/ocaml/libs/xb/xb.mli +++ b/tools/ocaml/libs/xb/xb.mli @@ -65,7 +65,7 @@ type backend_mmap = { } type backend_fd = { fd : Unix.file_descr; } type backend = Fd of backend_fd | Xenmmap of backend_mmap -type partial_buf = HaveHdr of Partial.pkt | NoHdr of int * string +type partial_buf = HaveHdr of Partial.pkt | NoHdr of int * bytes type t = { backend : backend; pkt_in : Packet.t Queue.t; @@ -76,10 +76,10 @@ type t = { val init_partial_in : unit -> partial_buf val reconnect : t -> unit val queue : t -> Packet.t -> unit -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 val write_mmap : backend_mmap -> 'a -> string -> int -> int val write : t -> string -> int -> int val output : t -> bool diff --git a/tools/ocaml/xenstored/logging.ml b/tools/ocaml/xenstored/logging.ml index 0c0d03d0c4..e3c769fb2c 100644 --- a/tools/ocaml/xenstored/logging.ml +++ b/tools/ocaml/xenstored/logging.ml @@ -60,11 +60,11 @@ type logger = let truncate_line nb_chars line = if String.length line > nb_chars - 1 then let len = max (nb_chars - 1) 2 in - let dst_line = String.create len in - String.blit line 0 dst_line 0 (len - 2); - dst_line.[len-2] <- '.'; - dst_line.[len-1] <- '.'; - dst_line + let dst_line = Bytes.create len in + Bytes.blit_string line 0 dst_line 0 (len - 2); + Bytes.set dst_line (len-2) '.'; + Bytes.set dst_line (len-1) '.'; + Bytes.to_string dst_line else line let log_rotate ref_ch log_file log_nb_files = @@ -252,13 +252,13 @@ let string_of_access_type = function *) let sanitize_data data = - let data = String.copy data in - for i = 0 to String.length data - 1 + let data = Bytes.copy data in + for i = 0 to Bytes.length data - 1 do - if data.[i] = '\000' then - data.[i] <- ' ' + if Bytes.get data i = '\000' then + Bytes.set data i ' ' done; - String.escaped data + String.escaped (Bytes.to_string data) let activate_access_log = ref true let access_log_destination = ref (File (Paths.xen_log_dir ^ "/xenstored-access.log")) @@ -291,7 +291,7 @@ let access_logging ~con ~tid ?(data="") ~level access_type = let date = string_of_date() in let tid = string_of_tid ~con tid in let access_type = string_of_access_type access_type in - let data = sanitize_data data in + let data = sanitize_data (Bytes.of_string data) in let prefix = prefix !access_log_destination date in let msg = Printf.sprintf "%s %s %s %s" prefix tid access_type data in logger.write ~level msg) diff --git a/tools/ocaml/xenstored/stdext.ml b/tools/ocaml/xenstored/stdext.ml index b8a8fd00e1..41411ee535 100644 --- a/tools/ocaml/xenstored/stdext.ml +++ b/tools/ocaml/xenstored/stdext.ml @@ -122,7 +122,7 @@ let pidfile_write filename = let pid = Unix.getpid () in let buf = string_of_int pid ^ "\n" in let len = String.length buf in - if Unix.write fd buf 0 len <> len + if Unix.write fd (Bytes.of_string buf) 0 len <> len then failwith "pidfile_write failed"; ) (fun () -> Unix.close fd) diff --git a/tools/ocaml/xenstored/utils.ml b/tools/ocaml/xenstored/utils.ml index e89c1aff04..5fcb042351 100644 --- a/tools/ocaml/xenstored/utils.ml +++ b/tools/ocaml/xenstored/utils.ml @@ -45,23 +45,23 @@ let get_hierarchy path = let hexify s = let hexseq_of_char c = sprintf "%02x" (Char.code c) in - let hs = String.create (String.length s * 2) in + let hs = Bytes.create (String.length s * 2) in for i = 0 to String.length s - 1 do let seq = hexseq_of_char s.[i] in - hs.[i * 2] <- seq.[0]; - hs.[i * 2 + 1] <- seq.[1]; + Bytes.set hs (i * 2) seq.[0]; + Bytes.set hs (i * 2 + 1) seq.[1]; done; - hs + Bytes.to_string hs let unhexify hs = let char_of_hexseq seq0 seq1 = Char.chr (int_of_string (sprintf "0x%c%c" seq0 seq1)) in - let s = String.create (String.length hs / 2) in - for i = 0 to String.length s - 1 + let b = Bytes.create (String.length hs / 2) in + for i = 0 to Bytes.length b - 1 do - s.[i] <- char_of_hexseq hs.[i * 2] hs.[i * 2 + 1] + Bytes.set b i (char_of_hexseq hs.[i * 2] hs.[i * 2 + 1]) done; - s + Bytes.to_string b let trim_path path = try @@ -84,10 +84,10 @@ 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 buf = Bytes.make 20 (char_of_int 0) in let sz = Unix.read fd buf 0 20 in Unix.close fd; - int_of_string (String.sub buf 0 sz) + int_of_string (Bytes.to_string (Bytes.sub buf 0 sz)) let path_complete path connection_path = if String.get path 0 <> '/' then -- 2.14.3 [-- Attachment #3: Type: text/plain, Size: 157 bytes --] _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] make xen ocaml safe-strings compliant 2018-03-12 19:35 ` Michael Young @ 2018-03-13 9:29 ` Christian Lindig 2018-03-13 14:49 ` Wei Liu 0 siblings, 1 reply; 18+ messages in thread From: Christian Lindig @ 2018-03-13 9:29 UTC (permalink / raw) To: Michael Young Cc: Wei Liu, John Thomson, Dario Faggioli, Marcello Seri, David Scott, Xen-devel On 12/03/18 19:35, Michael Young wrote: > 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 > 0001-make-xen-ocaml-safe-strings-compliant.patch > > From da088e4eef2bbea4be262e12db4c36960ff5145a Mon Sep 17 00:00:00 2001 > From: Michael Young<m.a.young@durham.ac.uk> > Date: Mon, 12 Mar 2018 18:49:29 +0000 > Subject: [PATCH v4] make xen ocaml safe-strings compliant > > Xen built with ocaml 4.06 gives errors such as > Error: This expression has type bytes but an expression was > expected of type string > as Byte and safe-strings which were introduced in 4.02 are the > default in 4.06. > This patch which is partly by Richard W.M. Jones of Red Hat > fromhttps://bugzilla.redhat.com/show_bug.cgi?id=1526703 > fixes these issues. > > v4: Where string s is now bytes, rename it to b. > > Signed-off-by: Michael Young<m.a.young@durham.ac.uk> > --- > tools/ocaml/libs/xb/xb.ml | 34 ++++++++++++++++++---------------- > tools/ocaml/libs/xb/xb.mli | 10 +++++----- > tools/ocaml/xenstored/logging.ml | 22 +++++++++++----------- > tools/ocaml/xenstored/stdext.ml | 2 +- > tools/ocaml/xenstored/utils.ml | 20 ++++++++++---------- > 5 files changed, 45 insertions(+), 43 deletions(-) Reviewed-by: Christian Lindig<christian.lindig@citrix.com> This patch is good to go. One caveat, though: the conversion between bytes and strings is not just a cast but involves copying and thus has a performance cost which we want to keep in mind. The OCaml low-level libraries like Unix.read return (mutable) bytes, but most code expects (immutable) strings. I believe that it would be difficult to avoid this in the long run and this patch is the right way to go. -- Christian _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] make xen ocaml safe-strings compliant 2018-03-13 9:29 ` Christian Lindig @ 2018-03-13 14:49 ` Wei Liu 0 siblings, 0 replies; 18+ messages in thread From: Wei Liu @ 2018-03-13 14:49 UTC (permalink / raw) To: Christian Lindig Cc: Wei Liu, David Scott, John Thomson, Dario Faggioli, Marcello Seri, Michael Young, Xen-devel On Tue, Mar 13, 2018 at 09:29:49AM +0000, Christian Lindig wrote: > On 12/03/18 19:35, Michael Young wrote: > > > 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 > > 0001-make-xen-ocaml-safe-strings-compliant.patch > > > > From da088e4eef2bbea4be262e12db4c36960ff5145a Mon Sep 17 00:00:00 2001 > > From: Michael Young<m.a.young@durham.ac.uk> > > Date: Mon, 12 Mar 2018 18:49:29 +0000 > > Subject: [PATCH v4] make xen ocaml safe-strings compliant > > > > Xen built with ocaml 4.06 gives errors such as > > Error: This expression has type bytes but an expression was > > expected of type string > > as Byte and safe-strings which were introduced in 4.02 are the > > default in 4.06. > > This patch which is partly by Richard W.M. Jones of Red Hat > > fromhttps://bugzilla.redhat.com/show_bug.cgi?id=1526703 > > fixes these issues. > > > > v4: Where string s is now bytes, rename it to b. > > > > Signed-off-by: Michael Young<m.a.young@durham.ac.uk> > > --- > > tools/ocaml/libs/xb/xb.ml | 34 ++++++++++++++++++---------------- > > tools/ocaml/libs/xb/xb.mli | 10 +++++----- > > tools/ocaml/xenstored/logging.ml | 22 +++++++++++----------- > > tools/ocaml/xenstored/stdext.ml | 2 +- > > tools/ocaml/xenstored/utils.ml | 20 ++++++++++---------- > > 5 files changed, 45 insertions(+), 43 deletions(-) > > Reviewed-by: Christian Lindig<christian.lindig@citrix.com> Thanks. I will apply this patch soon. Wei. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] make xen ocaml safe-strings compliant 2018-02-09 9:20 ` Christian Lindig 2018-02-09 10:06 ` Dario Faggioli 2018-02-12 14:55 ` Wei Liu @ 2018-02-13 0:35 ` Michael Young 2 siblings, 0 replies; 18+ messages in thread From: Michael Young @ 2018-02-13 0:35 UTC (permalink / raw) To: Christian Lindig Cc: Wei Liu, John Thomson, Dario Faggioli, Marcello Seri, David Scott, Xen-devel [-- Attachment #1: Type: text/plain, Size: 744 bytes --] On Fri, 9 Feb 2018, Christian Lindig wrote: > Sorry, I can’t make a promise because of my other obligations. I do > wonder, though: this patch did not come out of nowhere but supposedly > was working - what is different here? The patch was from Fedora and is broken there too! It fixes the build issues but it doesn't look like anyone tested it (I use xenstored). I have been trying to narrow down where the problem is and I think there are actually two issues; one in tools/ocaml/xenstored/utils.ml which I think causes an error like Fatal error: exception Failure("int_of_string") and one in tools/ocaml/libs/xb/xb.ml which I think causes an error like Fatal error: exception Failure("evtchn bind_interdomain failed") Michael Young [-- Attachment #2: Type: text/plain, Size: 157 bytes --] _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2018-03-13 15:09 UTC | newest] Thread overview: 18+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-01-30 22:55 [PATCH 1/2] make xen ocaml safe-strings compliant Michael Young 2018-02-06 16:49 ` Wei Liu 2018-02-06 21:56 ` Michael Young 2018-02-07 10:31 ` Wei Liu 2018-02-08 17:49 ` Dario Faggioli 2018-02-08 18:03 ` Wei Liu 2018-02-08 18:24 ` Wei Liu 2018-02-09 9:20 ` Christian Lindig 2018-02-09 10:06 ` Dario Faggioli 2018-02-12 14:55 ` Wei Liu 2018-03-09 22:57 ` Michael Young 2018-03-09 23:47 ` Christian Lindig 2018-03-10 0:36 ` Michael Young 2018-03-12 11:29 ` Christian Lindig 2018-03-12 19:35 ` Michael Young 2018-03-13 9:29 ` Christian Lindig 2018-03-13 14:49 ` Wei Liu 2018-02-13 0:35 ` Michael Young
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).