[PATCH 1/2] Remove most instances of OCaml warning 52.
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: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
On Fri, Dec 09, 2016 at 10:32:42AM +0100, Pino Toscano wrote:
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?
Even better is ... diff --git a/v2v/output_vdsm.ml b/v2v/output_vdsm.ml index a78e3e6..c3be84d 100644 --- a/v2v/output_vdsm.ml +++ b/v2v/output_vdsm.ml @@ -81,10 +81,9 @@ object let mp, uuid = let fields = String.nsplit "/" os in (* ... "data-center" "UUID" *) let fields = List.rev fields in (* "UUID" "data-center" ... *) + let fields = dropwhile ((=) "") fields in match fields with - | "" :: uuid :: rest (* handles trailing "/" case *) - | uuid :: rest - when String.length uuid = 36 -> + | uuid :: rest when String.length uuid = 36 -> let mp = String.concat "/" (List.rev rest) in mp, uuid | _ -> Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-builder quickly builds VMs from scratch http://libguestfs.org/virt-builder.1.html
On Friday, 9 December 2016 11:29:39 CET Richard W.M. Jones wrote:
On Fri, Dec 09, 2016 at 10:32:42AM +0100, Pino Toscano wrote:
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?
Even better is ...
diff --git a/v2v/output_vdsm.ml b/v2v/output_vdsm.ml index a78e3e6..c3be84d 100644 --- a/v2v/output_vdsm.ml +++ b/v2v/output_vdsm.ml @@ -81,10 +81,9 @@ object let mp, uuid = let fields = String.nsplit "/" os in (* ... "data-center" "UUID" *) let fields = List.rev fields in (* "UUID" "data-center" ... *) + let fields = dropwhile ((=) "") fields in match fields with - | "" :: uuid :: rest (* handles trailing "/" case *) - | uuid :: rest - when String.length uuid = 36 -> + | uuid :: rest when String.length uuid = 36 -> let mp = String.concat "/" (List.rev rest) in mp, uuid | _ ->
LGTM. Thanks, -- Pino Toscano
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
participants (2)
-
Pino Toscano -
Richard W.M. Jones