--- common/mlpcre/pcre-c.c | 7 +++++-- common/mlpcre/pcre_tests.ml | 7 +++++++ 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/common/mlpcre/pcre-c.c b/common/mlpcre/pcre-c.c index 6fae0e6f4..da9b50d34 100644 --- a/common/mlpcre/pcre-c.c +++ b/common/mlpcre/pcre-c.c @@ -201,6 +201,7 @@ value guestfs_int_pcre_sub (value nv) { CAMLparam1 (nv); + int n = Int_val (nv); CAMLlocal1 (strv); int len; CLEANUP_FREE char *str = NULL; @@ -209,8 +210,10 @@ guestfs_int_pcre_sub (value nv) if (m == NULL) raise_pcre_error ("PCRE.sub called without calling PCRE.matches", 0); - len = pcre_get_substring (m->subject, m->vec, m->r, Int_val (nv), - (const char **) &str); + if (n < 0) + caml_invalid_argument ("PCRE.sub: n must be >= 0"); + + len = pcre_get_substring (m->subject, m->vec, m->r, n, (const char **) &str); if (len == PCRE_ERROR_NOSUBSTRING) caml_raise_not_found (); diff --git a/common/mlpcre/pcre_tests.ml b/common/mlpcre/pcre_tests.ml index e5214eab8..2b18f462f 100644 --- a/common/mlpcre/pcre_tests.ml +++ b/common/mlpcre/pcre_tests.ml @@ -69,6 +69,13 @@ let () = | PCRE.Error (msg, code) -> failwith (sprintf "PCRE error: %s (PCRE error code %d)" msg code) +(* Run some out of range [sub] calls to ensure an exception is thrown. *) +let () = + let re2 = compile "(a+)(b*)" in + ignore (matches re2 "ccac"); + (try ignore (sub 3) with Not_found -> ()); + (try ignore (sub (-1)) with Invalid_argument _ -> ()) + (* Compile some bad regexps and check that an exception is thrown. * It would be nice to check the error message is right but * that involves dealing with language and future changes of -- 2.13.2

--- common/mlpcre/PCRE.ml | 3 +-- common/mlpcre/PCRE.mli | 14 ++++++++++++++ common/mlpcre/pcre-c.c | 27 +++++++++++++++++++++++++++ common/mlpcre/pcre_tests.ml | 19 ++++++++++++++++--- 4 files changed, 58 insertions(+), 5 deletions(-) diff --git a/common/mlpcre/PCRE.ml b/common/mlpcre/PCRE.ml index 94eea4b34..5269d41f8 100644 --- a/common/mlpcre/PCRE.ml +++ b/common/mlpcre/PCRE.ml @@ -23,10 +23,9 @@ exception Error of string * int type regexp external compile : string -> regexp = "guestfs_int_pcre_compile" - external matches : regexp -> string -> bool = "guestfs_int_pcre_matches" - external sub : int -> string = "guestfs_int_pcre_sub" +external subi : int -> int * int = "guestfs_int_pcre_subi" let () = Callback.register_exception "PCRE.Error" (Error ("", 0)) diff --git a/common/mlpcre/PCRE.mli b/common/mlpcre/PCRE.mli index 331a50a9a..02f16d19d 100644 --- a/common/mlpcre/PCRE.mli +++ b/common/mlpcre/PCRE.mli @@ -77,3 +77,17 @@ val sub : int -> string If there was no nth substring then this raises [Not_found]. This can also raise {!Error} for other PCRE-related errors. *) + +val subi : int -> int * int +(** Return the nth substring (capture) matched by the previous call + to {!matches} in the current thread. + + This is the same as {!sub} but instead of copying the + substring out, it returns the indexes into the original string + of the first character of the substring and the first + character after the substring. + + (See pcreapi(3) section "How pcre_exec() returns captured substrings" + for exact details). + + If there was no nth substring then this raises [Not_found]. *) diff --git a/common/mlpcre/pcre-c.c b/common/mlpcre/pcre-c.c index da9b50d34..15775dad0 100644 --- a/common/mlpcre/pcre-c.c +++ b/common/mlpcre/pcre-c.c @@ -225,3 +225,30 @@ guestfs_int_pcre_sub (value nv) memcpy (String_val (strv), str, len); CAMLreturn (strv); } + +value +guestfs_int_pcre_subi (value nv) +{ + CAMLparam1 (nv); + int n = Int_val (nv); + CAMLlocal1 (rv); + struct last_match *m = gl_tls_get (last_match); + + if (m == NULL) + raise_pcre_error ("PCRE.subi called without calling PCRE.matches", 0); + + if (n < 0) + caml_invalid_argument ("PCRE.subi: n must be >= 0"); + + /* eg if there are 2 captures, m->r == 3, and valid values of n are + * 0, 1 or 2. + */ + if (n >= m->r) + caml_raise_not_found (); + + rv = caml_alloc (2, 0); + Store_field (rv, 0, Val_int (m->vec[n*2])); + Store_field (rv, 1, Val_int (m->vec[n*2+1])); + + CAMLreturn (rv); +} diff --git a/common/mlpcre/pcre_tests.ml b/common/mlpcre/pcre_tests.ml index 2b18f462f..316a4348e 100644 --- a/common/mlpcre/pcre_tests.ml +++ b/common/mlpcre/pcre_tests.ml @@ -34,6 +34,12 @@ let sub i = eprintf " %s\n%!" r; r +let subi i = + eprintf "PCRE.subi %d ->%!" i; + let i1, i2 = PCRE.subi i in + eprintf " (%d, %d)\n%!" i1 i2; + (i1, i2) + let () = try let re0 = compile "a+b" in @@ -62,19 +68,26 @@ let () = assert (matches re2 "ccac" = true); assert (sub 1 = "a"); assert (sub 2 = ""); - assert (sub 0 = "a") + assert (sub 0 = "a"); + assert (subi 0 = (2, 3)); + assert (subi 1 = (2, 3)); + assert (subi 2 = (3, 3)) with | Not_found -> failwith "one of the PCRE.sub functions unexpectedly raised Not_found" | PCRE.Error (msg, code) -> failwith (sprintf "PCRE error: %s (PCRE error code %d)" msg code) -(* Run some out of range [sub] calls to ensure an exception is thrown. *) +(* Run some out of range [sub] and [subi] calls to ensure an exception + * is thrown. + *) let () = let re2 = compile "(a+)(b*)" in ignore (matches re2 "ccac"); (try ignore (sub 3) with Not_found -> ()); - (try ignore (sub (-1)) with Invalid_argument _ -> ()) + (try ignore (sub (-1)) with Invalid_argument _ -> ()); + (try ignore (subi 3) with Not_found -> ()); + (try ignore (subi (-1)) with Invalid_argument _ -> ()) (* Compile some bad regexps and check that an exception is thrown. * It would be nice to check the error message is right but -- 2.13.2

Similar to Perl s/// but lacks backreferences. --- common/mlpcre/PCRE.ml | 25 +++++++++++++++++++++++++ common/mlpcre/PCRE.mli | 13 +++++++++++++ common/mlpcre/pcre_tests.ml | 23 ++++++++++++++++++++++- 3 files changed, 60 insertions(+), 1 deletion(-) diff --git a/common/mlpcre/PCRE.ml b/common/mlpcre/PCRE.ml index 5269d41f8..0eb7eb2ec 100644 --- a/common/mlpcre/PCRE.ml +++ b/common/mlpcre/PCRE.ml @@ -27,5 +27,30 @@ external matches : regexp -> string -> bool = "guestfs_int_pcre_matches" external sub : int -> string = "guestfs_int_pcre_sub" external subi : int -> int * int = "guestfs_int_pcre_subi" +let rec replace ?(global = false) patt subst subj = + if not (matches patt subj) then + (* Return original string unchanged if patt doesn't match. *) + subj + else ( + (* If patt matches "yyyy" in the original string then we have + * the following situation, where "xxxx" is the part of the + * original string before the match, and "zzzz..." is the + * part after the match: + * "xxxxyyyyzzzzzzzzzzzzz" + * ^ ^ + * i1 i2 + *) + let i1, i2 = subi 0 in + let xs = String.sub subj 0 i1 (* "xxxx", part before the match *) in + let zs = String.sub subj i2 (String.length subj - i2) (* after *) in + + (* If the global flag was set, we want to continue substitutions + * in the rest of the string. + *) + let zs = if global then replace ~global patt subst zs else zs in + + xs ^ subst ^ zs + ) + let () = Callback.register_exception "PCRE.Error" (Error ("", 0)) diff --git a/common/mlpcre/PCRE.mli b/common/mlpcre/PCRE.mli index 02f16d19d..634cc600c 100644 --- a/common/mlpcre/PCRE.mli +++ b/common/mlpcre/PCRE.mli @@ -91,3 +91,16 @@ val subi : int -> int * int for exact details). If there was no nth substring then this raises [Not_found]. *) + +val replace : ?global:bool -> regexp -> string -> string -> string +(** [replace ?global patt subst subj] performs a search and replace + on the subject string ([subj]). Where [patt] matches the + string, [subst] is substituted. This works similarly to the + Perl function [s///]. + + The [?global] flag defaults to false, so only the first + instance of [patt] in the string is replaced. If set to true + then every instance of [patt] in the string is replaced. + + Note that this function does not allow backreferences. + Any captures in [patt] are ignored. *) diff --git a/common/mlpcre/pcre_tests.ml b/common/mlpcre/pcre_tests.ml index 316a4348e..b5f712d20 100644 --- a/common/mlpcre/pcre_tests.ml +++ b/common/mlpcre/pcre_tests.ml @@ -28,6 +28,12 @@ let matches re str = eprintf " %b\n%!" r; r +let replace ?(global = false) patt subst subj = + eprintf "PCRE.replace global:%b <patt> %s %s ->%!" global subst subj; + let r = PCRE.replace ~global patt subst subj in + eprintf " %s\n%!" r; + r + let sub i = eprintf "PCRE.sub %d ->%!" i; let r = PCRE.sub i in @@ -45,6 +51,7 @@ let () = let re0 = compile "a+b" in let re1 = compile "(a+)b" in let re2 = compile "(a+)(b*)" in + let re3 = compile "[^A-Za-z0-9_]" in assert (matches re0 "ccaaabbbb" = true); assert (sub 0 = "aaab"); @@ -71,7 +78,21 @@ let () = assert (sub 0 = "a"); assert (subi 0 = (2, 3)); assert (subi 1 = (2, 3)); - assert (subi 2 = (3, 3)) + assert (subi 2 = (3, 3)); + + assert (replace re0 "dd" "abcabcaabccca" = "ddcabcaabccca"); + assert (replace ~global:true re0 "dd" "abcabcaabccca" = "ddcddcddccca"); + + (* This example copies a usage from customize/firstboot.ml + * "\xc2\xa3" is utf-8 for the GBP sign. Ideally PCRE would + * recognize that this is a single character, however doing that + * would involve passing the PCRE_UTF8 flag when compiling + * patterns, and that could be problematic if PCRE was built + * without Unicode support (XXX). + *) + assert (replace ~global:true re3 "-" "this is a\xc2\xa3funny.name?" + (* = "this-is-a-funny-name-" if UTF-8 worked *) + = "this-is-a--funny-name-"); with | Not_found -> failwith "one of the PCRE.sub functions unexpectedly raised Not_found" -- 2.13.2

This idea was previously proposed by Tomáš Golembiovský in https://www.redhat.com/archives/libguestfs/2017-January/msg00138.html --- common/mlstdutils/std_utils.ml | 28 ++++++++++++++++------------ common/mlstdutils/std_utils.mli | 11 ++++++++--- common/mlstdutils/std_utils_tests.ml | 29 +++++++++++++++++++++++++++++ 3 files changed, 53 insertions(+), 15 deletions(-) diff --git a/common/mlstdutils/std_utils.ml b/common/mlstdutils/std_utils.ml index b731b8fd5..37eef0348 100644 --- a/common/mlstdutils/std_utils.ml +++ b/common/mlstdutils/std_utils.ml @@ -147,18 +147,7 @@ module String = struct done; if not !r then s else Bytes.to_string b2 - let rec nsplit sep str = - let len = length str in - let seplen = length sep in - let i = find str sep in - if i = -1 then [str] - else ( - let s' = sub str 0 i in - let s'' = sub str (i+seplen) (len-i-seplen) in - s' :: nsplit sep s'' - ) - - let split sep str = + let rec split sep str = let len = length sep in let seplen = length str in let i = find str sep in @@ -167,6 +156,21 @@ module String = struct sub str 0 i, sub str (i + len) (seplen - i - len) ) + and nsplit ?(max = 0) sep str = + if max < 0 then + invalid_arg "String.nsplit: max parameter should not be negative"; + + (* If we reached the limit, OR if the pattern does not match the string + * at all, return the rest of the string as a single element list. + *) + if max = 1 || find str sep = -1 then + [str] + else ( + let s1, s2 = split sep str in + let max = if max = 0 then 0 else max - 1 in + s1 :: nsplit ~max sep s2 + ) + let rec lines_split str = let buf = Buffer.create 16 in let len = length str in diff --git a/common/mlstdutils/std_utils.mli b/common/mlstdutils/std_utils.mli index d217e48d4..c08e51360 100644 --- a/common/mlstdutils/std_utils.mli +++ b/common/mlstdutils/std_utils.mli @@ -88,14 +88,19 @@ module String : sig [str] with [s2]. *) val replace_char : string -> char -> char -> string (** Replace character in string. *) - val nsplit : string -> string -> string list - (** [nsplit sep str] splits [str] into multiple strings at each - separator [sep]. *) val split : string -> string -> string * string (** [split sep str] splits [str] at the first occurrence of the separator [sep], returning the part before and the part after. If separator is not found, return the whole string and an empty string. *) + val nsplit : ?max:int -> string -> string -> string list + (** [nsplit ?max sep str] splits [str] into multiple strings at each + separator [sep]. + + As with the Perl split function, you can give an optional + [?max] parameter to limit the number of strings returned. The + final element of the list will contain the remainder of the + input string. *) val lines_split : string -> string list (** [lines_split str] splits [str] into lines, keeping continuation characters (i.e. [\] at the end of lines) into account. *) diff --git a/common/mlstdutils/std_utils_tests.ml b/common/mlstdutils/std_utils_tests.ml index ce49c7606..dcd237dab 100644 --- a/common/mlstdutils/std_utils_tests.ml +++ b/common/mlstdutils/std_utils_tests.ml @@ -18,6 +18,8 @@ (* This file tests the Std_utils module. *) +open Printf + open OUnit2 open Std_utils @@ -26,6 +28,7 @@ let assert_equal_string = assert_equal ~printer:(fun x -> x) let assert_equal_int = assert_equal ~printer:(fun x -> string_of_int x) let assert_equal_int64 = assert_equal ~printer:(fun x -> Int64.to_string x) let assert_equal_stringlist = assert_equal ~printer:(fun x -> "(" ^ (String.escaped (String.concat "," x)) ^ ")") +let assert_equal_stringpair = assert_equal ~printer:(fun (x, y) -> sprintf "%S, %S" x y) let test_subdirectory ctx = assert_equal_string "" (subdirectory "/foo" "/foo"); @@ -83,6 +86,30 @@ let test_string_find ctx = assert_equal_int (-1) (String.find "" "baz"); assert_equal_int (-1) (String.find "foobar" "baz") +(* Test Std_utils.String.split. *) +let test_string_split ctx = + assert_equal_stringpair ("a", "b") (String.split " " "a b"); + assert_equal_stringpair ("", "ab") (String.split " " " ab"); + assert_equal_stringpair ("", "abc") (String.split "" "abc"); + assert_equal_stringpair ("abc", "") (String.split " " "abc"); + assert_equal_stringpair ("", "") (String.split " " "") + +(* Test Std_utils.String.nsplit. *) +let test_string_nsplit ctx = + (* XXX Not clear if the next test case indicates an error in + * String.nsplit. However this is how it has historically worked. + *) + assert_equal_stringlist [""] (String.nsplit " " ""); + assert_equal_stringlist ["abc"] (String.nsplit " " "abc"); + assert_equal_stringlist ["a"; "b"; "c"] (String.nsplit " " "a b c"); + assert_equal_stringlist ["a"; "b"; "c"; ""] (String.nsplit " " "a b c "); + assert_equal_stringlist [""; "a"; "b"; "c"] (String.nsplit " " " a b c"); + assert_equal_stringlist [""; "a"; "b"; "c"; ""] (String.nsplit " " " a b c "); + assert_equal_stringlist ["a b c d"] (String.nsplit ~max:1 " " "a b c d"); + assert_equal_stringlist ["a"; "b c d"] (String.nsplit ~max:2 " " "a b c d"); + assert_equal_stringlist ["a"; "b"; "c d"] (String.nsplit ~max:3 " " "a b c d"); + assert_equal_stringlist ["a"; "b"; "c"; "d"] (String.nsplit ~max:10 " " "a b c d") + (* Test Std_utils.String.lines_split. *) let test_string_lines_split ctx = assert_equal_stringlist [""] (String.lines_split ""); @@ -129,6 +156,8 @@ let suite = "strings.is_prefix" >:: test_string_is_prefix; "strings.is_suffix" >:: test_string_is_suffix; "strings.find" >:: test_string_find; + "strings.split" >:: test_string_split; + "strings.nsplit" >:: test_string_nsplit; "strings.lines_split" >:: test_string_lines_split; "strings.span" >:: test_string_span; "strings.chomp" >:: test_string_chomp; -- 2.13.2

--- valgrind-suppressions | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/valgrind-suppressions b/valgrind-suppressions index 1c4b35355..2bc70659d 100644 --- a/valgrind-suppressions +++ b/valgrind-suppressions @@ -171,6 +171,17 @@ fun:caml_build_primitive_table } +# The OCaml PCRE.compile function calls pcre_compile2 which allocates +# memory. If these regexps are stored in a global variable then they +# can never be freed. This is not really a bug, so ignore it. +{ + ocaml_pcre_globals + Memcheck:Leak + fun:malloc + fun:pcre_compile2 + ... +} + # glibc { glibc_cond -- 2.13.2

--- customize/Makefile.am | 3 +++ customize/firstboot.ml | 6 +++--- customize/ssh_key.ml | 7 +++---- sysprep/Makefile.am | 3 +++ v2v/Makefile.am | 4 ++++ 5 files changed, 16 insertions(+), 7 deletions(-) diff --git a/customize/Makefile.am b/customize/Makefile.am index 18f4dce44..ff2c2e2d0 100644 --- a/customize/Makefile.am +++ b/customize/Makefile.am @@ -126,6 +126,7 @@ OCAMLPACKAGES = \ -I $(top_builddir)/ocaml \ -I $(top_builddir)/common/mlstdutils \ -I $(top_builddir)/common/mlutils \ + -I $(top_builddir)/common/mlpcre \ -I $(top_builddir)/mllib \ -I $(builddir) if HAVE_OCAML_PKG_GETTEXT @@ -155,6 +156,7 @@ endif OCAMLLINKFLAGS = \ mlstdutils.$(MLARCHIVE) \ mlguestfs.$(MLARCHIVE) \ + mlpcre.$(MLARCHIVE) \ mlcutils.$(MLARCHIVE) \ mllib.$(MLARCHIVE) \ customize.$(MLARCHIVE) \ @@ -176,6 +178,7 @@ virt_customize_DEPENDENCIES = \ $(CUSTOMIZE_THEOBJECTS) \ $(CUSTOMIZE_CMA) \ ../common/mlutils/mlcutils.$(MLARCHIVE) \ + ../common/mlpcre/mlpcre.$(MLARCHIVE) \ ../mllib/mllib.$(MLARCHIVE) virt_customize_LINK = \ $(top_srcdir)/ocaml-link.sh -cclib '$(OCAMLCLIBS)' -- \ diff --git a/customize/firstboot.ml b/customize/firstboot.ml index b07bda001..25203cf91 100644 --- a/customize/firstboot.ml +++ b/customize/firstboot.ml @@ -25,12 +25,12 @@ open Common_gettext.Gettext open Regedit let unix2dos s = - String.concat "\r\n" (Str.split_delim (Str.regexp_string "\n") s) + String.concat "\r\n" (String.nsplit "\n" s) let sanitize_name = - let rex = Str.regexp "[^A-Za-z0-9_]" in + let rex = PCRE.compile ~caseless:true "[^a-z0-9_]" in fun n -> - let n = Str.global_replace rex "-" n in + let n = PCRE.replace ~global:true rex "-" n in let len = String.length n and max = 60 in if len >= max then String.sub n 0 max else n diff --git a/customize/ssh_key.ml b/customize/ssh_key.ml index 185536d1d..da0e7d90c 100644 --- a/customize/ssh_key.ml +++ b/customize/ssh_key.ml @@ -47,8 +47,8 @@ and parse_selector_list orig_arg = function (* Find the local [on the host] user's SSH public key. See * ssh-copy-id(1) default_ID_file for rationale. *) -let pubkey_re = Str.regexp "^id.*\\.pub$" -let pubkey_ignore_re = Str.regexp ".*-cert\\.pub$" +let pubkey_re = PCRE.compile "^id.*\\.pub$" +let pubkey_ignore_re = PCRE.compile ".*-cert\\.pub$" let local_user_ssh_pubkey () = let home_dir = @@ -60,8 +60,7 @@ let local_user_ssh_pubkey () = let files = Array.to_list files in let files = List.filter ( fun file -> - Str.string_match pubkey_re file 0 && - not (Str.string_match pubkey_ignore_re file 0) + PCRE.matches pubkey_re file && not (PCRE.matches pubkey_ignore_re file) ) files in if files = [] then error (f_"ssh-inject: no public key file found in %s") ssh_dir; diff --git a/sysprep/Makefile.am b/sysprep/Makefile.am index 400b3c1a3..c1aca2966 100644 --- a/sysprep/Makefile.am +++ b/sysprep/Makefile.am @@ -117,6 +117,7 @@ OCAMLPACKAGES = \ -I $(top_builddir)/common/visit/.libs \ -I $(top_builddir)/common/mlstdutils \ -I $(top_builddir)/common/mlutils \ + -I $(top_builddir)/common/mlpcre \ -I $(top_builddir)/common/mlvisit \ -I $(top_builddir)/mllib \ -I $(top_builddir)/customize @@ -145,6 +146,7 @@ endif OCAMLLINKFLAGS = \ mlstdutils.$(MLARCHIVE) \ mlguestfs.$(MLARCHIVE) \ + mlpcre.$(MLARCHIVE) \ mlcutils.$(MLARCHIVE) \ mllib.$(MLARCHIVE) \ mlvisit.$(MLARCHIVE) \ @@ -155,6 +157,7 @@ virt_sysprep_DEPENDENCIES = \ $(OBJECTS) \ ../common/mlstdutils/mlstdutils.$(MLARCHIVE) \ ../common/mlutils/mlcutils.$(MLARCHIVE) \ + ../common/mlpcre/mlpcre.$(MLARCHIVE) \ ../mllib/mllib.$(MLARCHIVE) \ ../customize/customize.$(MLARCHIVE) \ $(top_srcdir)/ocaml-link.sh diff --git a/v2v/Makefile.am b/v2v/Makefile.am index 0aafc5725..3a38b3a98 100644 --- a/v2v/Makefile.am +++ b/v2v/Makefile.am @@ -151,6 +151,7 @@ OCAMLPACKAGES = \ -I $(top_builddir)/ocaml \ -I $(top_builddir)/common/mlstdutils \ -I $(top_builddir)/common/mlutils \ + -I $(top_builddir)/common/mlpcre \ -I $(top_builddir)/common/mlxml \ -I $(top_builddir)/mllib \ -I $(top_builddir)/customize @@ -177,6 +178,7 @@ endif OCAMLLINKFLAGS = \ mlstdutils.$(MLARCHIVE) \ mlguestfs.$(MLARCHIVE) \ + mlpcre.$(MLARCHIVE) \ mlxml.$(MLARCHIVE) \ mlcutils.$(MLARCHIVE) \ mllib.$(MLARCHIVE) \ @@ -218,6 +220,7 @@ virt_v2v_copy_to_local_DEPENDENCIES = \ $(COPY_TO_LOCAL_OBJECTS) \ ../common/mlstdutils/mlstdutils.$(MLARCHIVE) \ ../common/mlxml/mlxml.$(MLARCHIVE) \ + ../common/mlpcre/mlpcre.$(MLARCHIVE) \ ../common/mlutils/mlcutils.$(MLARCHIVE) \ ../mllib/mllib.$(MLARCHIVE) \ $(top_srcdir)/ocaml-link.sh @@ -508,6 +511,7 @@ v2v_unit_tests_DEPENDENCIES = \ $(v2v_unit_tests_THEOBJECTS) \ ../common/mlstdutils/mlstdutils.$(MLARCHIVE) \ ../common/mlxml/mlxml.$(MLARCHIVE) \ + ../common/mlpcre/mlpcre.$(MLARCHIVE) \ ../common/mlutils/mlcutils.$(MLARCHIVE) \ ../mllib/mllib.$(MLARCHIVE) \ $(top_srcdir)/ocaml-link.sh -- 2.13.2

In the original conversion of virt-v2v from Perl (commit 0131d6f666d93dd353c4e4092fd945861646d319), the Perl regular expression was incorrectly transscribed. ‘\b’ was not converted to ‘\\b’ so the new regexp was looking for an ASCII BEL character, not a word boundary. To fix this problem I converted the code to use PCRE, and went back to the original virt-v2v code (virt-v2v.git: lib/Sys/VirtConvert/Converter/Linux.pm)to find out what the Perl regular expression should have been. Note I have also removed ‘.*’ at the beginning and end of the regexp because PCRE regexps are not anchored. --- v2v/convert_linux.ml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/v2v/convert_linux.ml b/v2v/convert_linux.ml index 3e7652716..f78d18d39 100644 --- a/v2v/convert_linux.ml +++ b/v2v/convert_linux.ml @@ -115,10 +115,10 @@ let rec convert (g : G.guestfs) inspect source output rcaps = (try let lines = g#read_lines "/etc/rc.local" in let lines = Array.to_list lines in - let rex = Str.regexp ".*\\b\\(insmod|modprobe\\)\b.*\\bxen-vbd.*" in + let rex = PCRE.compile "\\b(insmod|modprobe)\\b.*\\bxen-vbd" in let lines = List.map ( fun s -> - if Str.string_match rex s 0 then + if PCRE.matches rex s then "#" ^ s else s -- 2.13.2

The old virt-v2v code ignored boot kernels with names like "/boot/vmlinuz-*.rpmsave". The transscribed code did not because the Str module requires ‘|’ to be escaped as ‘\|’. This changes the code to use PCRE and fixes it. --- v2v/linux_bootloaders.ml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/v2v/linux_bootloaders.ml b/v2v/linux_bootloaders.ml index 210cce762..00cb5cd19 100644 --- a/v2v/linux_bootloaders.ml +++ b/v2v/linux_bootloaders.ml @@ -319,9 +319,9 @@ object (self) Array.to_list (g#glob_expand "/boot/kernel-*") @ Array.to_list (g#glob_expand "/boot/vmlinuz-*") @ Array.to_list (g#glob_expand "/vmlinuz-*") in - let rex = Str.regexp ".*\\.\\(dpkg-.*|rpmsave|rpmnew\\)$" in + let rex = PCRE.compile "\\.(?:dpkg-.*|rpmsave|rpmnew)$" in let vmlinuzes = List.filter ( - fun file -> not (Str.string_match rex file 0) + fun file -> not (PCRE.matches rex file) ) vmlinuzes in vmlinuzes -- 2.13.2

For each regular expression I went back to the original Perl code to ensure that the regexp is correct. --- v2v/convert_linux.ml | 61 +++++++++++++++++------------------------------- v2v/linux_bootloaders.ml | 32 ++++++++++++------------- v2v/linux_kernels.ml | 17 +++++++------- 3 files changed, 46 insertions(+), 64 deletions(-) diff --git a/v2v/convert_linux.ml b/v2v/convert_linux.ml index 93a6f1732..df7d4dd14 100644 --- a/v2v/convert_linux.ml +++ b/v2v/convert_linux.ml @@ -176,11 +176,11 @@ let rec convert (g : G.guestfs) inspect source output rcaps = if g#is_file ~followsymlinks:true vboxconfig then ( let lines = g#read_lines vboxconfig in let lines = Array.to_list lines in - let rex = Str.regexp "^INSTALL_DIR=\\(.*\\)$" in + let rex = PCRE.compile "^INSTALL_DIR=(.*)$" in let lines = filter_map ( fun line -> - if Str.string_match rex line 0 then ( - let path = Str.matched_group 1 line in + if PCRE.matches rex line then ( + let path = PCRE.sub 1 in let path = shell_unquote path in if String.length path >= 1 && path.[0] = '/' then ( let vboxuninstall = path ^ "/uninstall.sh" in @@ -260,8 +260,7 @@ let rec convert (g : G.guestfs) inspect source output rcaps = *) if provides <> [] then ( (* Trim whitespace. *) - let rex = Str.regexp "^[ \t]*\\([^ \t]+\\)[ \t]*$" in - let provides = List.map (Str.replace_first rex "\\1") provides in + let provides = List.map String.trim provides in (* Install the dependencies with yum. Use yum explicitly * because we don't have package names and local install is @@ -327,7 +326,7 @@ let rec convert (g : G.guestfs) inspect source output rcaps = * ttys in /etc/inittab if the system uses it. We need to put * them back. *) - let rex = Str.regexp "^\\([1-6]\\):\\([2-5]+\\):respawn:\\(.*\\)" in + let rex = PCRE.compile "^([1-6]):([2-5]+):respawn:(.*)" in let updated = ref false in let rec loop () = let comments = g#aug_match "/files/etc/inittab/#comment" in @@ -336,10 +335,10 @@ let rec convert (g : G.guestfs) inspect source output rcaps = | [] -> () | commentp :: _ -> let comment = g#aug_get commentp in - if Str.string_match rex comment 0 then ( - let name = Str.matched_group 1 comment in - let runlevels = Str.matched_group 2 comment in - let process = Str.matched_group 3 comment in + if PCRE.matches rex comment then ( + let name = PCRE.sub 1 + and runlevels = PCRE.sub 2 + and process = PCRE.sub 3 in if String.find process "getty" >= 0 then ( updated := true; @@ -628,14 +627,12 @@ let rec convert (g : G.guestfs) inspect source output rcaps = *) let paths = g#aug_match "/files/etc/inittab/*/process" in let paths = Array.to_list paths in - let rex = Str.regexp "\\(.*\\)\\b\\([xh]vc0\\)\\b\\(.*\\)" in + let rex = PCRE.compile "\\b([xh]vc0)\\b" in List.iter ( fun path -> let proc = g#aug_get path in - if Str.string_match rex proc 0 then ( - let proc = Str.global_replace rex "\\1ttyS0\\3" proc in - g#aug_set path proc - ); + let proc' = PCRE.replace ~global:true rex "ttyS0" proc in + if proc <> proc' then g#aug_set path proc' ) paths; let paths = g#aug_match "/files/etc/securetty/*" in @@ -658,11 +655,11 @@ let rec convert (g : G.guestfs) inspect source output rcaps = *) let paths = g#aug_match "/files/etc/inittab/*/process" in let paths = Array.to_list paths in - let rex = Str.regexp ".*\\b\\([xh]vc0|ttyS0\\)\\b.*" in + let rex = PCRE.compile "\\b([xh]vc0|ttyS0)\\b" in List.iter ( fun path -> let proc = g#aug_get path in - if Str.string_match rex proc 0 then + if PCRE.matches rex proc then ignore (g#aug_rm (path ^ "/..")) ) paths; @@ -918,14 +915,8 @@ let rec convert (g : G.guestfs) inspect source output rcaps = (* Map device names for each entry. *) let rex_resume = PCRE.compile "^(.*resume=)(/dev/\\S+)(.*)$" - and rex_device_cciss_p = - Str.regexp "^/dev/\\(cciss/c[0-9]+d[0-9]+\\)p\\([0-9]+\\)$" - and rex_device_cciss = - Str.regexp "^/dev/\\(cciss/c[0-9]+d[0-9]+\\)$" - and rex_device_p = - Str.regexp "^/dev/\\([a-z]+\\)\\([0-9]+\\)$" - and rex_device = - Str.regexp "^/dev/\\([a-z]+\\)$" in + and rex_device_cciss = PCRE.compile "^/dev/(cciss/c\\d+d\\d+)(?:p(\\d+))?$" + and rex_device = PCRE.compile "^/dev/([a-z]+)(\\d*)?$" in let rec replace_if_device path value = let replace device = @@ -952,24 +943,16 @@ let rec convert (g : G.guestfs) inspect source output rcaps = ) else value ) - else if Str.string_match rex_device_cciss_p value 0 then ( - let device = Str.matched_group 1 value - and part = Str.matched_group 2 value in + else if PCRE.matches rex_device_cciss value then ( + let device = PCRE.sub 1 + and part = try PCRE.sub 2 with Not_found -> "" in "/dev/" ^ replace device ^ part ) - else if Str.string_match rex_device_cciss value 0 then ( - let device = Str.matched_group 1 value in - "/dev/" ^ replace device - ) - else if Str.string_match rex_device_p value 0 then ( - let device = Str.matched_group 1 value - and part = Str.matched_group 2 value in + else if PCRE.matches rex_device value then ( + let device = PCRE.sub 1 + and part = try PCRE.sub 2 with Not_found -> "" in "/dev/" ^ replace device ^ part ) - else if Str.string_match rex_device value 0 then ( - let device = Str.matched_group 1 value in - "/dev/" ^ replace device - ) else (* doesn't look like a known device name *) value in diff --git a/v2v/linux_bootloaders.ml b/v2v/linux_bootloaders.ml index 00cb5cd19..59af38a86 100644 --- a/v2v/linux_bootloaders.ml +++ b/v2v/linux_bootloaders.ml @@ -44,9 +44,9 @@ type bootloader_type = | Grub2 (* Helper function for SUSE: remove (hdX,X) prefix from a path. *) -let remove_hd_prefix path = - let rex = Str.regexp "^(hd.*)\\(.*\\)" in - Str.replace_first rex "\\1" path +let remove_hd_prefix = + let rex = PCRE.compile "^\\(hd.*\\)" in + PCRE.replace rex "" (* Grub1 (AKA grub-legacy) representation. *) class bootloader_grub1 (g : G.guestfs) inspect grub_config = @@ -132,11 +132,11 @@ object if paths = [] then error (f_"didn't find grub entry for kernel %s") vmlinuz; let path = List.hd paths in - let rex = Str.regexp ".*/title\\[\\([1-9][0-9]*\\)\\]/kernel" in - if not (Str.string_match rex path 0) then + let rex = PCRE.compile "/title(?:\\[(\\d+)\\])?/kernel" in + if not (PCRE.matches rex path) then error (f_"internal error: regular expression did not match ‘%s’") path; - let index = int_of_string (Str.matched_group 1 path) - 1 in + let index = try int_of_string (PCRE.sub 1) - 1 with Not_found -> 0 in g#aug_set (sprintf "/files%s/default" grub_config) (string_of_int index); g#aug_save () @@ -151,7 +151,7 @@ object ) else false method configure_console () = - let rex = Str.regexp "\\(.*\\)\\b\\([xh]vc0\\)\\b\\(.*\\)" in + let rex = PCRE.compile "\\b([xh]vc0)\\b" in let expr = sprintf "/files%s/title/kernel/console" grub_config in let paths = g#aug_match expr in @@ -159,23 +159,21 @@ object List.iter ( fun path -> let console = g#aug_get path in - if Str.string_match rex console 0 then ( - let console = Str.global_replace rex "\\1ttyS0\\3" console in - g#aug_set path console - ) + let console' = PCRE.replace ~global:true rex "ttyS0" console in + if console <> console' then g#aug_set path console' ) paths; g#aug_save () method remove_console () = - let rex = Str.regexp "\\(.*\\)\\b\\([xh]vc0\\)\\b\\(.*\\)" in + let rex = PCRE.compile "\\b([xh]vc0)\\b" in let expr = sprintf "/files%s/title/kernel/console" grub_config in let rec loop = function | [] -> () | path :: paths -> let console = g#aug_get path in - if Str.string_match rex console 0 then ( + if PCRE.matches rex console then ( ignore (g#aug_rm path); (* All the paths are invalid, restart the loop. *) let paths = g#aug_match expr in @@ -231,7 +229,7 @@ object (self) inherit bootloader method private grub2_update_console ~remove () = - let rex = Str.regexp "\\(.*\\)\\bconsole=[xh]vc0\\b\\(.*\\)" in + let rex = PCRE.compile "\\bconsole=[xh]vc0\\b" in let paths = [ "/files/etc/sysconfig/grub/GRUB_CMDLINE_LINUX"; @@ -249,12 +247,12 @@ object (self) warning (f_"could not remove grub2 serial console (ignored)") | path :: _ -> let grub_cmdline = g#aug_get path in - if Str.string_match rex grub_cmdline 0 then ( + if PCRE.matches rex grub_cmdline then ( let new_grub_cmdline = if not remove then - Str.global_replace rex "\\1console=ttyS0\\2" grub_cmdline + PCRE.replace ~global:true rex "console=ttyS0" grub_cmdline else - Str.global_replace rex "\\1\\2" grub_cmdline in + PCRE.replace ~global:true rex "" grub_cmdline in g#aug_set path new_grub_cmdline; g#aug_save (); diff --git a/v2v/linux_kernels.ml b/v2v/linux_kernels.ml index 6e1ca4bf1..9853b0029 100644 --- a/v2v/linux_kernels.ml +++ b/v2v/linux_kernels.ml @@ -63,10 +63,12 @@ let print_kernel_info chan prefix ki = fpf "pvpanic=%b xen=%b debug=%b\n" ki.ki_supports_isa_pvpanic ki.ki_is_xen_pv_only_kernel ki.ki_is_debug +let rex_ko = PCRE.compile "\\.k?o(?:\\.xz)?$" +let rex_ko_extract = PCRE.compile "/([^/]+)\\.k?o(?:\\.xz)?$" + let detect_kernels (g : G.guestfs) inspect family bootloader = (* What kernel/kernel-like packages are installed on the current guest? *) let installed_kernels : kernel_info list = - let rex_ko = Str.regexp ".*\\.k?o\\(\\.xz\\)?$" in let check_config feature = function | None -> false | Some config -> @@ -82,12 +84,11 @@ let detect_kernels (g : G.guestfs) inspect family bootloader = | _ -> false ) in - let rex_ko_extract = Str.regexp ".*/\\([^/]+\\)\\.k?o\\(\\.xz\\)?$" in let rex_initrd = if family = `Debian_family then - Str.regexp "^initrd.img-.*$" + PCRE.compile "^initrd.img-.*$" else - Str.regexp "^initr\\(d\\|amfs\\)-.*\\(\\.img\\)?$" in + PCRE.compile "^initr(?:d|amfs)-.*(?:\\.img)?$" in filter_map ( function | { G.app2_name = name } as app @@ -133,7 +134,7 @@ let detect_kernels (g : G.guestfs) inspect family bootloader = let files = g#ls "/boot" in let files = Array.to_list files in let files = - List.filter (fun n -> Str.string_match rex_initrd n 0) files in + List.filter (fun n -> PCRE.matches rex_initrd n) files in let files = List.filter ( fun n -> @@ -165,7 +166,7 @@ let detect_kernels (g : G.guestfs) inspect family bootloader = let modules = g#find modpath in let modules = Array.to_list modules in let modules = - List.filter (fun m -> Str.string_match rex_ko m 0) modules in + List.filter (fun m -> PCRE.matches rex_ko m) modules in assert (List.length modules > 0); (* Determine the kernel architecture by looking at the @@ -178,8 +179,8 @@ let detect_kernels (g : G.guestfs) inspect family bootloader = (* Just return the module names, without path or extension. *) let modules = filter_map ( fun m -> - if Str.string_match rex_ko_extract m 0 then - Some (Str.matched_group 1 m) + if PCRE.matches rex_ko_extract m then + Some (PCRE.sub 1) else None ) modules in -- 2.13.2

On Friday, 22 September 2017 09:36:16 CEST Richard W.M. Jones wrote: TBH I'd simplify here using String.find: if (String.find dispname "Parallels Tools." = -1) && (String.find dispname "Virtuozzo Tools." = -1) then It should be simpler and faster. -- Pino Toscano

--- v2v/output_libvirt.ml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/v2v/output_libvirt.ml b/v2v/output_libvirt.ml index b5df8245f..bc7f41ff9 100644 --- a/v2v/output_libvirt.ml +++ b/v2v/output_libvirt.ml @@ -28,8 +28,8 @@ open Xpath_helpers open Create_libvirt_xml let arch_is_sane_or_die = - let rex = Str.regexp "^[-_A-Za-z0-9]+$" in - fun arch -> assert (Str.string_match rex arch 0) + let rex = PCRE.compile ~caseless:true "^[-_a-z0-9]+$" in + fun arch -> assert (PCRE.matches rex arch) let target_features_of_capabilities_doc doc arch = let xpathctx = Xml.xpath_new_context doc in -- 2.13.2

On Friday, 22 September 2017 09:36:19 CEST Richard W.M. Jones wrote: This can be optimized as "/{2,}", so single slashes will not be replaced by themselves. -- Pino Toscano

--- v2v/parse_vmx.ml | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/v2v/parse_vmx.ml b/v2v/parse_vmx.ml index 770dc29d3..271066eb9 100644 --- a/v2v/parse_vmx.ml +++ b/v2v/parse_vmx.ml @@ -226,7 +226,7 @@ and vmx_bool_of_string t = else failwith "bool_of_string" (* Regular expression used to match key = "value" in VMX file. *) -let rex = Str.regexp "^\\([^ \t=]+\\)[ \t]*=[ \t]*\"\\(.*\\)\"$" +let rex = PCRE.compile "^([^ \t=]+)\\s*=\\s*\"(.*)\"$" (* Remove the weird escapes used in value strings. See description above. *) let remove_vmx_escapes str = @@ -292,10 +292,9 @@ and parse_string str = (* Parse the lines into key = "value". *) let lines = filter_map ( fun line -> - if Str.string_match rex line 0 then ( - let key = Str.matched_group 1 line in + if PCRE.matches rex line then ( + let key = PCRE.sub 1 and value = PCRE.sub 2 in let key = String.lowercase_ascii key in - let value = Str.matched_group 2 line in let value = remove_vmx_escapes value in Some (key, value) ) -- 2.13.2

Updates commit 8f91d3a9b0356a701c1904e0a2efa5d272d08ac2 similar to Tomáš's original intended code. --- v2v/utils.ml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/v2v/utils.ml b/v2v/utils.ml index 008452d69..12ebe23f4 100644 --- a/v2v/utils.ml +++ b/v2v/utils.ml @@ -138,6 +138,8 @@ let backend_is_libvirt () = let backend = fst (String.split ":" backend) in backend = "libvirt" +let ws = PCRE.compile "\\s+" + let find_file_in_tar tar filename = let lines = external_command (sprintf "tar tRvf %s" (Filename.quote tar)) in let rec loop lines = @@ -147,7 +149,7 @@ let find_file_in_tar tar filename = (* Lines have the form: * block <offset>: <perms> <owner>/<group> <size> <mdate> <mtime> <file> *) - let elems = Str.bounded_split (Str.regexp " +") line 8 in + let elems = PCRE.nsplit ~max:8 ws line in if List.length elems = 8 && List.hd elems = "block" then ( let elems = Array.of_list elems in let offset = elems.(1) in -- 2.13.2