[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [Libguestfs] [PATCH 1/2] mllib: add and use last_part_of



On Wed, Jul 01, 2015 at 05:49:06PM +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.
> ---
>  customize/password.ml                     |  5 +++--
>  mllib/common_utils.ml                     |  7 +++++++
>  mllib/common_utils.mli                    |  3 +++
>  sysprep/sysprep_operation_user_account.ml |  5 +++--
>  v2v/convert_linux.ml                      | 10 +++-------
>  v2v/utils.ml                              | 16 ++++++++++------
>  6 files changed, 29 insertions(+), 17 deletions(-)
> 
> diff --git a/customize/password.ml b/customize/password.ml
> index 25ce901..d91c4b5 100644
> --- a/customize/password.ml
> +++ b/customize/password.ml
> @@ -98,8 +98,9 @@ let rec set_linux_passwords ?password_crypto (g : Guestfs.guestfs) root password
>    List.iter (
>      fun userpath ->
>        let user =
> -        let i = String.rindex userpath '/' in
> -        String.sub userpath (i+1) (String.length userpath -i-1) in
> +        match last_part_of userpath '/' with
> +        | Some x -> x
> +        | None -> error "password: missing '/' in %s" userpath in

Best to translate these strings:

  | None -> error (f_"password: missing '/' in %s") 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..083c5d5 100644
> --- a/mllib/common_utils.ml
> +++ b/mllib/common_utils.ml
> @@ -738,3 +738,10 @@ 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 =
> +  try
> +    let i = String.rindex str sep in
> +    Some (String.sub str (i+1) (String.length str - (i+1)))
> +  with Not_found -> None
> diff --git a/mllib/common_utils.mli b/mllib/common_utils.mli
> index a5b0a68..550f37f 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 option
> +(** 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..78c60d0 100644
> --- a/sysprep/sysprep_operation_user_account.ml
> +++ b/sysprep/sysprep_operation_user_account.ml
> @@ -69,8 +69,9 @@ let user_account_perform g root side_effects =
>          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
> +          match last_part_of userpath '/' with
> +          | Some x -> x
> +          | None -> error "user-accounts: missing '/' in %s" 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..f5a716f 100644
> --- a/v2v/convert_linux.ml
> +++ b/v2v/convert_linux.ml
> @@ -795,13 +795,9 @@ 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))
> -        with
> -          Not_found ->
> -            invalid_arg (sprintf "invalid module path: %s" modpath) in
> +        match last_part_of modpath '/' with
> +        | Some x -> x
> +        | None -> invalid_arg (sprintf "invalid module path: %s" modpath) in

That should probably be 'error' actually, but the bug already exists :-(

>        if g#is_file ~followsymlinks:true "/sbin/dracut" then (
>          (* Dracut. *)
> diff --git a/v2v/utils.ml b/v2v/utils.ml
> index 2aeba45..976fe85 100644
> --- a/v2v/utils.ml
> +++ b/v2v/utils.ml
> @@ -176,9 +176,11 @@ 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 =
> +              match last_part_of path '/' with
> +              | Some x -> x
> +              | None ->
> +                error "v2v/find_virtio_win_drivers: missing '/' in %s" path in
>              (path, sprintf "%s:%s" virtio_win path,
>               basename,
>               fun () -> g#read_file path)
> @@ -199,9 +201,11 @@ let find_virtio_win_drivers virtio_win =
>            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
> +            match last_part_of lc_basename '.' with
> +            | Some x -> x
> +            | None ->
> +              error "v2v/find_virtio_win_drivers: missing '.' in %s"
> +                lc_basename in
>  
>            (* Skip files without specific extensions. *)
>            if extension <> "cat" && extension <> "inf" &&
> -- 
> 2.1.0

ACK with (f_...) around the first error parameter.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]