Re: [Qemu-devel] [PATCH v2 5/5] qmp: Add example usage of strto*l() qemu wrapper

[Carlos L. Torres]

On Sun, Jul 19, 2015 at 07:51:00PM +0200, Paolo Bonzini wrote:
> 
> 
> On 19/07/2015 01:52, Carlos L. Torres wrote:
> > +    int err;
> >  
> >      info->qemu = g_new0(VersionTriple, 1);
> > -    info->qemu->major = strtol(version, &tmp, 10);
> > +    err = qemu_strtol(version, &tmp, 10, &(info->qemu->major));
> 
> There are usually no parentheses around the argument of the & operator.
> 
> > +    if (err) {
> > +        error_setg(errp, "There was a problem retrieving QEMU major 
> > version.");
> > +    }
> 
> I think it's okay to just assert that err is zero.  Otherwise, this
> simple example is okay.  Thanks!
> 
> Paolo
> 
> >      tmp++;
> > -    info->qemu->minor = strtol(tmp, &tmp, 10);
> > +
> > +    err = qemu_strtol(tmp, &tmp, 10, &(info->qemu->minor));
> > +    if (err) {
> > +        error_setg(errp, "There was a problem retrieving QEMU minor 
> > version.");
> > +    }
> >      tmp++;
> > -    info->qemu->micro = strtol(tmp, &tmp, 10);
> > +
> > +    err = qemu_strtol(tmp, &tmp, 10, &(info->qemu->micro));
> > +    if (err) {
> > +        error_setg(errp, "There was a problem retrieving QEMU micro 
> > version.");
> > +    }

Hi Paolo,

Thanks for taking the time to review the patch.
I'll make those small changes, and post v3 version of the patch.

Thanks,
-- Carlos Torres