On Tue, Sep 19, 2017 at 04:28:12PM +0200, Pino Toscano wrote: > On Wednesday, 9 August 2017 19:23:43 CEST Richard W.M. Jones wrote: > > +let parse_version_from_major_minor str data = > > + if verbose () then > > + eprintf "parse_version_from_major_minor: parsing '%s'\n%!" str; > > + > > + if PCRE.matches re_major_minor str || > > + PCRE.matches re_major_no_minor str then ( > > + let major = > > + try Some (int_of_string (PCRE.sub 1)) > > + with Not_found | Invalid_argument _ | Failure _ -> None in > > + let minor = > > + try Some (int_of_string (PCRE.sub 2)) > > + with Not_found | Invalid_argument _ | Failure _ -> None in > > + match major, minor with > > + | None, None > > + | None, Some _ -> () > > + | Some major, None -> data.version <- Some (major, 0) > > + | Some major, Some minor -> data.version <- Some (major, minor) > > + ) > > IMHO this is more complex than needed: > > if PCRE.matches re_major_minor str ( > let major = int_of_string (PCRE.sub 1) in > let minor = int_of_string (PCRE.sub 2) in > data.version <- Some (major, minor) > ) > else if PCRE.matches re_major_no_minor str then ( > let major = int_of_string (PCRE.sub 1) in > data.version <- Some (major, 0) > ) > > After all, the regexps should already ensure the captures are > available, and that they caught integer values.
I'll send separate patches to fix/simplify these cases. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 100 libraries supported. http://fedoraproject.org/wiki/MinGW _______________________________________________ Libguestfs mailing list Libguestfs@redhat.com https://www.redhat.com/mailman/listinfo/libguestfs