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

Pino Toscano ptoscano at redhat.com
Fri Dec 9 12:09:06 UTC 2016


On Friday, 9 December 2016 11:29:39 CET Richard W.M. Jones wrote:
> 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
>        | _ ->

LGTM.

Thanks,
-- 
Pino Toscano
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <http://listman.redhat.com/archives/libguestfs/attachments/20161209/e0bd3ad3/attachment.sig>


More information about the Libguestfs mailing list