[Libguestfs] sscanf/stroul (was: Re: [PATCH nbdkit v3 2/3] Add new retry filter.)

[Richard W.M. Jones]

On Thu, Sep 19, 2019 at 11:32:47AM -0500, Eric Blake wrote:
> > +static int
> > +retry_config (nbdkit_next_config *next, void *nxdata,
> > +              const char *key, const char *value)
> > +{
> > +  int r;
> > +
> > +  if (strcmp (key, "retries") == 0) {
> > +    if (sscanf (value, "%d", &retries) != 1 || retries < 0) {
> > +      nbdkit_error ("cannot parse retries: %s", value);
> > +      return -1;
> > +    }
> > +  }
> > +  else if (strcmp (key, "retry-delay") == 0) {
> > +    if (sscanf (value, "%d", &initial_delay) != 1 || initial_delay <= 0) {
> 
> sscanf("%d") cannot detect overflow; should this use strtol with errno
> checking instead?

AIUI glibc does detect overflow, but it's the C17/POSIX standards
which don't mandate it?

I had a longer email here where I was going to suggest replacing all
use of sscanf/strtoul with new nbdkit_parse_* functions, but we would
need a lot of them and they still wouldn't cover all the clever uses
we make of sscanf, eg:

  https://github.com/libguestfs/nbdkit/blob/cfbbe17ee21a9a47a96a5ac7f4321d20ed90cad9/filters/error/error.c#L134

So I still think the right way to solve this is to fix C2x / POSIX so
it mandates what is possibly already the glibc behaviour.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v