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

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

On Monday, 16 October 2017 18:15:38 CEST Richard W.M. Jones
wrote:
> 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).
Yes, this is what patch #2 already does, in parse_os_release:

+     let values = split_key_value_strings ~unquote:shell_unquote lines in

simple_unquote is used for parse_lsb_release: I could not find any
standard documentation for its format, and the examples I have have
either no quoting, or double quoting.
> > +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.
OK, I will rename it to parse_key_value_strings (since the "_file"
suffix would imply it parses a file, while it just acts on strings).

Thanks,
-- 
Pino Toscano

On Mon, Oct 16, 2017 at 06:29:08PM +0200, Pino Toscano
wrote:
> Yes, this is what patch #2 already does, in parse_os_release:
> 
> +     let values = split_key_value_strings ~unquote:shell_unquote lines in
> 
> simple_unquote is used for parse_lsb_release: I could not find any
> standard documentation for its format, and the examples I have have
> either no quoting, or double quoting.
Oh right, sorry I missed that.  I've ACKed the v3 version.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/