Re: [Libguestfs] [PATCH 2/2] v2v: Fix ambiguous and probably incorrect pattern match (warning 57).

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 | _ ->