Re: [Libguestfs] [PATCH] mllib: Getopt: fix integer parsing

On Mon, Jul 18, 2016 at 10:13:30AM +0200, Pino Toscano wrote: > Since we are using gnulib already, make use of xstrtol to parse the > integer arguments to avoid extra suffixes, etc. > > Fixes commit 0f7bf8f714898c606e5d5015fff5b7803dcd1aee. > --- > mllib/getopt-c.c | 34 ++++++++++++++++++++++++---------- > 1 file changed, 24 insertions(+), 10 deletions(-) > > diff --git a/mllib/getopt-c.c b/mllib/getopt-c.c > index 1f129a7..2d3f9b6 100644 > --- a/mllib/getopt-c.c > +++ b/mllib/getopt-c.c > @@ -30,6 +30,8 @@ > #include <error.h> > #include <assert.h> > > +#include "xstrtol.h" > + > #include <caml/alloc.h> > #include <caml/fail.h> > #include <caml/memory.h> > @@ -117,6 +119,26 @@ do_call1 (value funv, value paramv) > CAMLreturn0; > } This function needs to return something other than 'int', since on 64 bit OCaml integers (the final destination) are signed 63 bits. I think returning 'long' is a better idea, and the receiving 'num' should also be 'long' (as in my patch).

> +static int > +strtoint (const char *arg) > +{ > + long int num; > + > + if (xstrtol (arg, NULL, 0, &num, NULL) != LONGINT_OK) { > + fprintf (stderr, _("%s: '%s' is not a numeric value.\n"), > + guestfs_int_program_name, arg); > + show_error (EXIT_FAILURE); > + } > + > + if (num <= INT_MIN || num >= INT_MAX) { > + fprintf (stderr, _("%s: %s: integer out of range\n"), > + guestfs_int_program_name, arg); > + show_error (EXIT_FAILURE); These bounds are not tight enough. On 32 bit they should check the range of a 31 bit signed int, and on 64 bit, a 63 bit signed int.