[Libguestfs] [PATCH] mllib: Getopt: fix integer parsing
Pino Toscano
ptoscano at redhat.com
Mon Jul 18 08:47:12 UTC 2016
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
-------------- 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/20160718/ff2e480b/attachment.sig>
More information about the Libguestfs
mailing list