[Libguestfs] [PATCH 2/3] mllib: add and use last_part_of
Richard W.M. Jones
rjones at redhat.com
Wed Jul 1 13:50:16 UTC 2015
On Wed, Jul 01, 2015 at 03:12:54PM +0200, Pino Toscano wrote:
> Collect this small snippet to get the part of a string after the last
> occurrency of a character; replace with it the current snippets doing
> the same.
>
> Should be just code motion.
I see a future where we get a customer bug that some utility fails
with a mysterious 'Not_found' exception and no stack trace.
At the very least we should document that last_part_of can raise
Not_found (because of String.rindex). Even better would be if
last_part_of returns None | Some substring, which would force all the
callers to check for this case and deal with it.
I know the code may already have this bug, but best to kill it now.
Rich.
> customize/password.ml | 4 +---
> mllib/common_utils.ml | 5 +++++
> mllib/common_utils.mli | 3 +++
> sysprep/sysprep_operation_user_account.ml | 4 +---
> v2v/convert_linux.ml | 4 +---
> v2v/utils.ml | 9 ++-------
> 6 files changed, 13 insertions(+), 16 deletions(-)
>
> diff --git a/customize/password.ml b/customize/password.ml
> index 25ce901..cb97804 100644
> --- a/customize/password.ml
> +++ b/customize/password.ml
> @@ -97,9 +97,7 @@ let rec set_linux_passwords ?password_crypto (g : Guestfs.guestfs) root password
> let users = Array.to_list (g#aug_ls "/files/etc/shadow") in
> List.iter (
> fun userpath ->
> - let user =
> - let i = String.rindex userpath '/' in
> - String.sub userpath (i+1) (String.length userpath -i-1) in
> + let user = last_part_of userpath '/' in
> try
> (* Each line is: "user:[!!]password:..."
> * !! at the front of the password field means the account is locked.
> diff --git a/mllib/common_utils.ml b/mllib/common_utils.ml
> index 3737b4c..8601009 100644
> --- a/mllib/common_utils.ml
> +++ b/mllib/common_utils.ml
> @@ -738,3 +738,8 @@ let guest_arch_compatible guest_arch =
> | x, y when x = y -> true
> | "x86_64", ("i386"|"i486"|"i586"|"i686") -> true
> | _ -> false
> +
> +(** Return the last part of a string, after the specified separator. *)
> +let last_part_of str sep =
> + let i = String.rindex str sep in
> + String.sub str (i+1) (String.length str - (i+1))
> diff --git a/mllib/common_utils.mli b/mllib/common_utils.mli
> index a5b0a68..7d0b38f 100644
> --- a/mllib/common_utils.mli
> +++ b/mllib/common_utils.mli
> @@ -178,3 +178,6 @@ val mkdir_p : string -> int -> unit
> val guest_arch_compatible : string -> bool
> (** Are guest arch and host_cpu compatible, in terms of being able
> to run commands in the libguestfs appliance? *)
> +
> +val last_part_of : string -> char -> string
> +(** Return the last part of a string, after the specified separator. *)
> diff --git a/sysprep/sysprep_operation_user_account.ml b/sysprep/sysprep_operation_user_account.ml
> index 3f3b142..fe460b8 100644
> --- a/sysprep/sysprep_operation_user_account.ml
> +++ b/sysprep/sysprep_operation_user_account.ml
> @@ -68,9 +68,7 @@ let user_account_perform g root side_effects =
> let uid = userpath ^ "/uid" in
> let uid = g#aug_get uid in
> let uid = int_of_string uid in
> - let username =
> - let i = String.rindex userpath '/' in
> - String.sub userpath (i+1) (String.length userpath -i-1) in
> + let username = last_part_of userpath '/' in
> if uid >= uid_min && uid <= uid_max
> && check_remove_user username then (
> changed := true;
> diff --git a/v2v/convert_linux.ml b/v2v/convert_linux.ml
> index 7967c0f..cadd515 100644
> --- a/v2v/convert_linux.ml
> +++ b/v2v/convert_linux.ml
> @@ -795,10 +795,8 @@ let rec convert ~keep_serial_console (g : G.guestfs) inspect source =
> *)
> let mkinitrd_kv =
> let modpath = kernel.ki_modpath in
> - let len = String.length modpath in
> try
> - let i = String.rindex modpath '/' in
> - String.sub modpath (i+1) (len - (i+1))
> + last_part_of modpath '/'
> with
> Not_found ->
> invalid_arg (sprintf "invalid module path: %s" modpath) in
> diff --git a/v2v/utils.ml b/v2v/utils.ml
> index 1f8c357..4d6b08d 100644
> --- a/v2v/utils.ml
> +++ b/v2v/utils.ml
> @@ -176,9 +176,7 @@ let find_virtio_win_drivers virtio_win =
> let paths = List.filter (g#is_file ~followsymlinks:false) paths in
> List.map (
> fun path ->
> - let i = String.rindex path '/' in
> - let len = String.length path in
> - let basename = String.sub path (i+1) (len - (i+1)) in
> + let basename = last_part_of path '/' in
> (path, sprintf "%s:%s" virtio_win path,
> basename,
> fun () -> g#read_file path)
> @@ -198,10 +196,7 @@ let find_virtio_win_drivers virtio_win =
> let lc_path = String.lowercase path in
> let lc_basename = String.lowercase basename in
>
> - let extension =
> - let i = String.rindex lc_basename '.' in
> - let len = String.length lc_basename in
> - String.sub lc_basename (i+1) (len - (i+1)) in
> + let extension = last_part_of lc_basename '.' in
>
> (* Skip files without specific extensions. *)
> if extension <> "cat" && extension <> "inf" &&
> --
> 2.1.0
>
> _______________________________________________
> Libguestfs mailing list
> Libguestfs at 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
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW
More information about the Libguestfs
mailing list