On Monday, 18 July 2016 09:38:43 CEST Richard W.M. Jones wrote: > 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).
I still don't understand why we need to handle values bigger than int
(as in C int, i.e. signed 32 bits) at all -- neither it is actually
needed, nor it would be coherent in 32bit vs 64bit builds.
If we really 64bit values as parameters, then let's just create a new
option type marking that.
>
> > +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.
Right, so -(2LL<<30) to (2LL<<30)-1 -- updating the patch accordingly.
Thanks,
--
Pino Toscano
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ Libguestfs mailing list [email protected] https://www.redhat.com/mailman/listinfo/libguestfs
