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

[Richard W.M. Jones]

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

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html