Libguestfs - Oct 2017 - Re: [PATCH v2 1/2] daemon: add split_key_value_strings helper

Changes from v1 to v2:
- split the "simple unquoting" as helper
- pass the unquoting function to split_key_value_strings
- use right unquoting function when applying split_key_value_strings

Pino Toscano (2):
  daemon: add split_key_value_strings helper
  daemon: use split_key_value_strings

 daemon/inspect_fs_unix.ml | 93 +++++++++++++++++++----------------------------
 daemon/md.ml              |  9 ++---
 daemon/utils.ml           | 16 ++++++++
 daemon/utils.mli          | 11 ++++++
 4 files changed, 67 insertions(+), 62 deletions(-)

-- 
2.13.6

Add a simple helper to turn a list of strings into key/value pairs,
splitting by '=', with the possibility to apply a function to unquote
values.

Add also a simple unquote function.
---
 daemon/utils.ml  | 16 ++++++++++++++++
 daemon/utils.mli | 11 +++++++++++
 2 files changed, 27 insertions(+)

diff --git a/daemon/utils.ml b/daemon/utils.ml
index d87ad75db..865936280 100644
--- a/daemon/utils.ml
+++ b/daemon/utils.ml
@@ -229,3 +229,19 @@ let unix_canonical_path path    let path = String.nsplit
"/" path in
   let path = List.filter ((<>) "") path in
   (if is_absolute then "/" else "") ^ String.concat
"/" path
+
+let simple_unquote s +  let n = String.length s in
+  if n >= 2 &&
+     ((s.[0] = '"' && s.[n-1] = '"') ||
(s.[0] = '\'' && s.[n-1] = '\'')) then
+    String.sub s 1 (n-2)
+  else
+    s
+
+let split_key_value_strings ?unquote lines +  let lines = List.filter
((<>) "") lines in
+  let lines = List.filter (fun s -> s.[0] <> '#') lines in
+  let lines = List.map (String.split "=") lines in
+  match unquote with
+  | None -> lines
+  | Some f -> List.map (fun (k, v) -> (k, f v)) lines
diff --git a/daemon/utils.mli b/daemon/utils.mli
index f312bde41..c44a6dc76 100644
--- a/daemon/utils.mli
+++ b/daemon/utils.mli
@@ -97,5 +97,16 @@ val unix_canonical_path : string -> string
     The path is modified in place because the result is always
     the same length or shorter than the argument passed. *)
 
+val simple_unquote : string -> string
+(** Unquote the string, by removing a pair of single- or double-quotes
+    at the beginning and the end of the string.
+
+    No other handling is done, unlike what {!shell_unquote} does. *)
+
+val split_key_value_strings : ?unquote:(string -> string) -> string list
-> (string * string) list
+(** Split the lines by the [=] separator; if [unquote] is specified,
+    it is applied on the values as unquote function.  Empty lines,
+    or that start with a comment character [#], are ignored. *)
+
 (**/**)
 val get_verbose_flag : unit -> bool
-- 
2.13.6

Make use of split_key_value_strings to slightly simplify the parsing of
mdadm output, and the parsing of os-release, and lsb-release files.
In particular, the parsing of lsb-release is simplified a lot, bringing
it much like the parsing of os-release.
---
 daemon/inspect_fs_unix.ml | 93 +++++++++++++++++++----------------------------
 daemon/md.ml              |  9 ++---
 2 files changed, 40 insertions(+), 62 deletions(-)

diff --git a/daemon/inspect_fs_unix.ml b/daemon/inspect_fs_unix.ml
index 234fccc2b..58c9be59e 100644
--- a/daemon/inspect_fs_unix.ml
+++ b/daemon/inspect_fs_unix.ml
@@ -73,31 +73,20 @@ let rec parse_os_release release_file data    match lines
with
   | None -> false
   | Some lines ->
+     let values = split_key_value_strings ~unquote:shell_unquote lines in
      List.iter (
-       fun line ->
-         let line = String.trim line in
-         if line = "" || line.[0] = '#' then
-           ()
-         else (
-           let key, value = String.split "=" line in
-           let value -             let n = String.length value in
-             if n >= 2 && value.[0] = '"' &&
value.[n-1] = '"' then
-               String.sub value 1 (n-2)
-             else
-               value in
-           if key = "ID" then (
-             let distro = distro_of_os_release_id value in
-             match distro with
-             | Some _ as distro -> data.distro <- distro
-             | None -> ()
-           )
-           else if key = "PRETTY_NAME" then
-             data.product_name <- Some value
-           else if key = "VERSION_ID" then
-             parse_os_release_version_id value data
+       fun (key, value) ->
+         if key = "ID" then (
+           let distro = distro_of_os_release_id value in
+           match distro with
+           | Some _ as distro -> data.distro <- distro
+           | None -> ()
          )
-       ) lines;
+         else if key = "PRETTY_NAME" then
+           data.product_name <- Some value
+         else if key = "VERSION_ID" then
+           parse_os_release_version_id value data
+       ) values;
 
      (* If we haven't got all the fields, exit right away. *)
      if data.distro = None || data.product_name = None then
@@ -204,48 +193,40 @@ and parse_lsb_release release_file data        *)
      let ok = ref false in
 
+     let values = split_key_value_strings ~unquote:simple_unquote lines in
      List.iter (
-       fun line ->
+       fun (key, value) ->
          if verbose () then
-           eprintf "parse_lsb_release: parsing: %s\n%!" line;
+           eprintf "parse_lsb_release: parsing: %s=%s\n%!" key value;
 
-         if data.distro = None && line = "DISTRIB_ID=Ubuntu"
then (
-           ok := true;
-           data. distro <- Some DISTRO_UBUNTU
-         )
-         else if data.distro = None && line =
"DISTRIB_ID=LinuxMint" then (
-           ok := true;
-           data.distro <- Some DISTRO_LINUX_MINT
-         )
-         else if data.distro = None && line =
"DISTRIB_ID=\"Mageia\"" then (
-           ok := true;
-           data.distro <- Some DISTRO_MAGEIA
-         )
-         else if data.distro = None && line =
"DISTRIB_ID=CoreOS" then (
-           ok := true;
-           data.distro <- Some DISTRO_COREOS
-         )
-         else if String.is_prefix line "DISTRIB_RELEASE=" then (
-           let line = String.sub line 16 (String.length line - 16) in
-           parse_version_from_major_minor line data
-         )
-         else if String.is_prefix line "DISTRIB_DESCRIPTION=\""
||
-                 String.is_prefix line "DISTRIB_DESCRIPTION='"
then (
-           ok := true;
-           let n = String.length line in
-           let product_name = String.sub line 21 (n-22) in
-           data.product_name <- Some product_name
+         if key = "DISTRIB_ID" then (
+           let distro = distro_of_lsb_release_distrib_id value in
+           match distro with
+           | Some _ as distro ->
+             ok := true;
+             data.distro <- distro
+           | None -> ()
          )
-         else if String.is_prefix line "DISTRIB_DESCRIPTION=" then (
+         else if key = "DISTRIB_RELEASE" then
+           parse_version_from_major_minor value data
+         else if key = "DISTRIB_DESCRIPTION" then (
            ok := true;
-           let n = String.length line in
-           let product_name = String.sub line 20 (n-20) in
-           data.product_name <- Some product_name
+           data.product_name <- Some value
          )
-     ) lines;
+     ) values;
 
      !ok
 
+(* DISTRIB_ID="Ubuntu" => Some DISTRO_UBUNTU *)
+and distro_of_lsb_release_distrib_id = function
+  | "CoreOS" -> Some DISTRO_COREOS
+  | "LinuxMint" -> Some DISTRO_LINUX_MINT
+  | "Mageia" -> Some DISTRO_MAGEIA
+  | "Ubuntu" -> Some DISTRO_UBUNTU
+  | value ->
+     eprintf "lsb-release: unknown DISTRIB_ID=%s\n" value;
+     None
+
 and parse_suse_release release_file data    let chroot = Chroot.create
~name:"parse_suse_release" () in
   let lines = Chroot.f chroot (fun () -> read_small_file release_file) () in
diff --git a/daemon/md.ml b/daemon/md.ml
index 5a40a2d83..68d7ea757 100644
--- a/daemon/md.ml
+++ b/daemon/md.ml
@@ -53,7 +53,6 @@ let md_detail md    (* Split the command output into lines. *)
   let out = String.trim out in
   let lines = String.nsplit "\n" out in
-  let lines = List.filter ((<>) "") lines in
 
   (* Parse the output of mdadm -D --export:
    * MD_LEVEL=raid1
@@ -62,11 +61,9 @@ let md_detail md     *
MD_UUID=cfa81b59:b6cfbd53:3f02085b:58f4a2e1
    * MD_NAME=localhost.localdomain:0
    *)
+  let values = split_key_value_strings lines in
   List.map (
-    fun line ->
-      (* Split the line at the equals sign. *)
-      let key, value = String.split "=" line in
-
+    fun (key, value) ->
       (* Remove the MD_ prefix from the key and translate the
        * remainder to lower case.
        *)
@@ -79,4 +76,4 @@ let md_detail md  
       (* Add the key/value pair to the output. *)
       (key, value)
-  ) lines
+  ) values
-- 
2.13.6

On Mon, Oct 16, 2017 at 05:58:10PM +0200, Pino Toscano
wrote:
> Add a simple helper to turn a list of strings into key/value pairs,
> splitting by '=', with the possibility to apply a function to
unquote
> values.
> 
> Add also a simple unquote function.
> ---
>  daemon/utils.ml  | 16 ++++++++++++++++
>  daemon/utils.mli | 11 +++++++++++
>  2 files changed, 27 insertions(+)
> 
> diff --git a/daemon/utils.ml b/daemon/utils.ml
> index d87ad75db..865936280 100644
> --- a/daemon/utils.ml
> +++ b/daemon/utils.ml
> @@ -229,3 +229,19 @@ let unix_canonical_path path >    let path =
String.nsplit "/" path in
>    let path = List.filter ((<>) "") path in
>    (if is_absolute then "/" else "") ^ String.concat
"/" path
> +
> +let simple_unquote s > +  let n = String.length s in
> +  if n >= 2 &&
> +     ((s.[0] = '"' && s.[n-1] = '"') ||
(s.[0] = '\'' && s.[n-1] = '\'')) then
> +    String.sub s 1 (n-2)
> +  else
> +    s
According to:

  https://www.freedesktop.org/software/systemd/man/os-release.html

os-release uses some kind of escaping system.  It does look as if
shell_unquote may be appropriate here.  (Of course whether writers of
/etc/os-release are doing the right thing is another issue.  I guess
there is no validation).
> +let split_key_value_strings ?unquote lines 
Can we call this function something like ‘parse_key_value_file’?  Most
of our other parsing functions are called ‘parse_xxx’, where as
‘*split*’ functions are generally reserved for functions that split a
single string.

Rich.
> +  let lines = List.filter ((<>) "") lines in
> +  let lines = List.filter (fun s -> s.[0] <> '#') lines
in
> +  let lines = List.map (String.split "=") lines in
> +  match unquote with
> +  | None -> lines
> +  | Some f -> List.map (fun (k, v) -> (k, f v)) lines
> diff --git a/daemon/utils.mli b/daemon/utils.mli
> index f312bde41..c44a6dc76 100644
> --- a/daemon/utils.mli
> +++ b/daemon/utils.mli
> @@ -97,5 +97,16 @@ val unix_canonical_path : string -> string
>      The path is modified in place because the result is always
>      the same length or shorter than the argument passed. *)
>  
> +val simple_unquote : string -> string
> +(** Unquote the string, by removing a pair of single- or double-quotes
> +    at the beginning and the end of the string.
> +
> +    No other handling is done, unlike what {!shell_unquote} does. *)
> +
> +val split_key_value_strings : ?unquote:(string -> string) -> string
list -> (string * string) list
> +(** Split the lines by the [=] separator; if [unquote] is specified,
> +    it is applied on the values as unquote function.  Empty lines,
> +    or that start with a comment character [#], are ignored. *)
> +
>  (**/**)
>  val get_verbose_flag : unit -> bool
> -- 
> 2.13.6
> 
> _______________________________________________
> Libguestfs mailing list
> Libguestfs@redhat.com
> https://www.redhat.com/mailman/listinfo/libguestfs
-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v