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 :-(