[Libguestfs] [PATCH] generator: Share Common_utils code.

[Pino Toscano]

On Thursday, 8 December 2016 10:36:45 CET Richard W.M. Jones wrote:
> For a very long time we have maintained two sets of utility functions,
> in mllib/common_utils.ml and generator/utils.ml.  This changes things
> so that the same set of utility functions can be shared with both
> directories.
> 
> It's not possible to use common_utils.ml directly in the generator
> because it provides several functions that use modules outside the
> OCaml stdlib.  Therefore we add some lightweight post-processing which
> extracts the functions using only the stdlib:
> 
>   (*<stdlib>*)
>   ...
>   (*</stdlib>*)

One idea here: instead of using a custom <stdlib> markup and sed code
for it, what about using the standard C preprocessor for this?
I.e. have comments like:

  (*
  #ifdef STDLIB
  *)
  ...
  (*
  #endif
  *)

and using cpp -DSTDLIB ... to output that.

> diff --git a/generator/utils.ml b/generator/utils.ml
> index 3e81433..ba5e045 100644
> --- a/generator/utils.ml
> +++ b/generator/utils.ml
> @@ -23,6 +23,8 @@
>   * makes this a bit harder than it should be.
>   *)
>  
> +open Common_utils
> +
>  open Unix
>  open Printf
>  
> @@ -119,85 +121,6 @@ let rstructs_used_by functions =
>  
>  let failwithf fs = ksprintf failwith fs
>  
> -let unique = let i = ref 0 in fun () -> incr i; !i

This seems to be used only in generator, so I'd leave it here for now
(reduces the changes in this patch, and it can always be moved later
on when needed).

> -let replace_char s c1 c2 =
> -  let b2 = Bytes.of_string s in
> -  let r = ref false in
> -  for i = 0 to Bytes.length b2 - 1 do
> -    if Bytes.unsafe_get b2 i = c1 then (
> -      Bytes.unsafe_set b2 i c2;
> -      r := true
> -    )
> -  done;
> -  if not !r then s else Bytes.to_string b2

Ditto.

> -let isspace c =
> -  c = ' '
> -  (* || c = '\f' *) || c = '\n' || c = '\r' || c = '\t' (* || c = '\v' *)

Ditto.

> -let triml ?(test = isspace) str =
> -  let i = ref 0 in
> -  let n = ref (String.length str) in
> -  while !n > 0 && test str.[!i]; do
> -    decr n;
> -    incr i
> -  done;
> -  if !i = 0 then str
> -  else String.sub str !i !n
> -
> -let trimr ?(test = isspace) str =
> -  let n = ref (String.length str) in
> -  while !n > 0 && test str.[!n-1]; do
> -    decr n
> -  done;
> -  if !n = String.length str then str
> -  else String.sub str 0 !n
> -
> -let trim ?(test = isspace) str =
> -  trimr ~test (triml ~test str)

Ditto.

> -let (|>) x f = f x

Ditto.

> -let rec find_map f = function
> -  | [] -> raise Not_found
> -  | x :: xs ->
> -      match f x with
> -      | Some y -> y
> -      | None -> find_map f xs

Ditto. Also it does not seem to be used anywhere.

> -let count_chars c str =
> -  let count = ref 0 in
> -  for i = 0 to String.length str - 1 do
> -    if c = String.unsafe_get str i then incr count
> -  done;
> -  !count

Ditto.

> -let explode str =
> -  let r = ref [] in
> -  for i = 0 to String.length str - 1 do
> -    let c = String.unsafe_get str i in
> -    r := c :: !r;
> -  done;
> -  List.rev !r

Ditto.

> -let map_chars f str =
> -  List.map f (explode str)

Ditto.

> -let spaces n = String.make n ' '

Oh there's a copy of this in mllib/getopt.ml, so could you please
include that in this patch?

> -
> -let html_escape text =
> -  let text = replace_str text "&" "&" in
> -  let text = replace_str text "<" "<" in
> -  let text = replace_str text ">" ">" in
> -  text

Not a big deal, but could this be left here to reduce the diff?

> diff --git a/mllib/common_utils.ml b/mllib/common_utils.ml
> index 78618f5..e1d1ab8 100644
> --- a/mllib/common_utils.ml
> +++ b/mllib/common_utils.ml
> @@ -16,7 +16,13 @@
>   * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
>   *)
>  
> +(* The parts between <stdlib>..</stdlib> are copied into the
> + * generator/common_utils.ml file.  These parts must ONLY use
> + * functions from the OCaml stdlib.
> + *)
> +(*<stdlib>*)
>  open Printf
> +(*</stdlib>*)

Theoretically this could stay in both version, couldn't it? This way
there are less changes between the mllib version and the generation one.

Thanks,
-- 
Pino Toscano
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <http://listman.redhat.com/archives/libguestfs/attachments/20161209/724529a7/attachment.sig>