Libguestfs - Dec 2016 - Re: [PATCH 2/2] v2v: Fix ambiguous and probably incorrect pattern match (warning 57).

See:
http://caml.inria.fr/pub/docs/manual-ocaml/comp.html#s:comp-warnings
---
 builder/index_parser.ml  | 8 ++++----
 generator/tests_c_api.ml | 4 ++--
 mllib/common_utils.ml    | 2 +-
 v2v/inspect_source.ml    | 2 +-
 v2v/linux.ml             | 2 +-
 v2v/xpath_helpers.ml     | 2 +-
 6 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/builder/index_parser.ml b/builder/index_parser.ml
index e5e4c6c..a3cae7d 100644
--- a/builder/index_parser.ml
+++ b/builder/index_parser.ml
@@ -113,7 +113,7 @@ let get_index ~downloader ~sigchecker
             try Rev_int (int_of_string (List.assoc ("revision", None)
fields))
             with
             | Not_found -> Rev_int 1
-            | Failure "int_of_string" ->
+            | Failure _ ->
               eprintf (f_"%s: cannot parse 'revision' field for
'%s'\n") prog n;
               corrupt_file () in
           let format @@ -124,7 +124,7 @@ let get_index ~downloader ~sigchecker
             | Not_found ->
               eprintf (f_"%s: no 'size' field for
'%s'\n") prog n;
               corrupt_file ()
-            | Failure "int_of_string" ->
+            | Failure _ ->
               eprintf (f_"%s: cannot parse 'size' field for
'%s'\n") prog n;
               corrupt_file () in
           let compressed_size @@ -132,7 +132,7 @@ let get_index ~downloader
~sigchecker
             with
             | Not_found ->
               None
-            | Failure "int_of_string" ->
+            | Failure _ ->
               eprintf (f_"%s: cannot parse 'compressed_size' field
for '%s'\n")
                 prog n;
               corrupt_file () in
@@ -157,7 +157,7 @@ let get_index ~downloader ~sigchecker
             try bool_of_string (List.assoc ("hidden", None) fields)
             with
             | Not_found -> false
-            | Failure "bool_of_string" ->
+            | Failure _ ->
               eprintf (f_"%s: cannot parse 'hidden' field for
'%s'\n")
                 prog n;
               corrupt_file () in
diff --git a/generator/tests_c_api.ml b/generator/tests_c_api.ml
index 8b98927..4a70433 100644
--- a/generator/tests_c_api.ml
+++ b/generator/tests_c_api.ml
@@ -557,13 +557,13 @@ and generate_test_command_call ?(expect_error = false)
?(do_return = true) ?test
     | Int _, arg, _ ->
       let i          try int_of_string arg
-        with Failure "int_of_string" ->
+        with Failure _ ->
           failwithf "%s: expecting an int, but got '%s'"
test_name arg in
       pr ", %d" i
     | Int64 _, arg, _ ->
       let i          try Int64.of_string arg
-        with Failure "int_of_string" ->
+        with Failure _ ->
           failwithf "%s: expecting an int64, but got '%s'"
test_name arg in
       pr ", %Ld" i
     | Bool _, arg, _ ->
diff --git a/mllib/common_utils.ml b/mllib/common_utils.ml
index f4ddf01..f948dce 100644
--- a/mllib/common_utils.ml
+++ b/mllib/common_utils.ml
@@ -730,7 +730,7 @@ let compare_version v1 v2            let rest =
Str.matched_group 2 str in
           let n              try `Number (int_of_string n)
-            with Failure "int_of_string" -> `String n in
+            with Failure _ -> `String n in
           n, rest
         )
         else if Str.string_match rex_letters str 0 then
diff --git a/v2v/inspect_source.ml b/v2v/inspect_source.ml
index cf8c98f..ae9940d 100644
--- a/v2v/inspect_source.ml
+++ b/v2v/inspect_source.ml
@@ -120,7 +120,7 @@ and choose_root root_choice g = function
             try i := int_of_string input
             with
             | End_of_file -> error (f_"connection closed")
-            | Failure "int_of_string" -> ()
+            | Failure _ -> ()
           )
         done;
         List.nth roots (!i - 1)
diff --git a/v2v/linux.ml b/v2v/linux.ml
index d449e10..729bb5d 100644
--- a/v2v/linux.ml
+++ b/v2v/linux.ml
@@ -145,7 +145,7 @@ let rec file_owner (g : G.guestfs) inspect path            
raise Not_found
          else
            raise exn
-       | Invalid_argument "index out of bounds" ->
+       | Invalid_argument _ (* pkgs.(0) raises index out of bounds *) ->
          error (f_"internal error: file_owner: rpm command returned no
output")
       )
 
diff --git a/v2v/xpath_helpers.ml b/v2v/xpath_helpers.ml
index 5d925fe..70af72da 100644
--- a/v2v/xpath_helpers.ml
+++ b/v2v/xpath_helpers.ml
@@ -31,7 +31,7 @@ let xpath_eval parsefn xpathctx expr      let node =
Xml.xpathobj_node obj 0 in
     let str = Xml.node_as_string node in
     try Some (parsefn str)
-    with Failure "int_of_string" ->
+    with Failure _ ->
       error (f_"expecting XML expression to return an integer (expression:
%s, matching string: %s)")
             expr str
   )
-- 
2.10.2

See:
http://caml.inria.fr/pub/docs/manual-ocaml/comp.html#ss%3Awarn57

I believe the code as written previously was incorrect.  However we
are lucky because if neither clause matches then it will fall through
to displaying an error message, allowing the user to correct the
problem.
---
 v2v/output_vdsm.ml | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/v2v/output_vdsm.ml b/v2v/output_vdsm.ml
index a78e3e6..fb7dd3c 100644
--- a/v2v/output_vdsm.ml
+++ b/v2v/output_vdsm.ml
@@ -83,6 +83,9 @@ object
       let fields = List.rev fields in      (* "UUID"
"data-center" ... *)
       match fields with
       | "" :: uuid :: rest                 (* handles trailing
"/" case *)
+          when String.length uuid = 36 ->
+        let mp = String.concat "/" (List.rev rest) in
+        mp, uuid
       | uuid :: rest
           when String.length uuid = 36 ->
         let mp = String.concat "/" (List.rev rest) in
-- 
2.10.2

On Thursday, 8 December 2016 13:54:03 CET Richard W.M. Jones
wrote:
> See:
> http://caml.inria.fr/pub/docs/manual-ocaml/comp.html#s:comp-warnings
> ---
This patch LGTM.

Thanks,
-- 
Pino Toscano

On Thursday, 8 December 2016 13:54:04 CET Richard W.M. Jones
wrote:
> See:
> http://caml.inria.fr/pub/docs/manual-ocaml/comp.html#ss%3Awarn57
> 
> I believe the code as written previously was incorrect.  However we
> are lucky because if neither clause matches then it will fall through
> to displaying an error message, allowing the user to correct the
> problem.
> ---
>  v2v/output_vdsm.ml | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/v2v/output_vdsm.ml b/v2v/output_vdsm.ml
> index a78e3e6..fb7dd3c 100644
> --- a/v2v/output_vdsm.ml
> +++ b/v2v/output_vdsm.ml
> @@ -83,6 +83,9 @@ object
>        let fields = List.rev fields in      (* "UUID"
"data-center" ... *)
>        match fields with
>        | "" :: uuid :: rest                 (* handles trailing
"/" case *)
> +          when String.length uuid = 36 ->
> +        let mp = String.concat "/" (List.rev rest) in
> +        mp, uuid
>        | uuid :: rest
>            when String.length uuid = 36 ->
>          let mp = String.concat "/" (List.rev rest) in
Rather than duplicating the (admittely small) code block, what about
removing the slash before splitting, so there is no need for the
separate match case handling the empty part?

-- 
Pino Toscano