Re: [Qemu-devel] [PATCH v10 2/3] quorum: implement bdrv_add_child() and bdrv_del_child()

[Changlong Xie]

On 03/08/2016 12:02 AM, Max Reitz wrote:
> On 07.03.2016 17:02, Eric Blake wrote:
> > On 03/05/2016 11:13 AM, Max Reitz wrote:
> >
> > > > +    index = atoi(child->name + 9);
> > >
> > > Optional: Assert absence of an error:
> >
> > Indeed, atoi() is worthless, because it cannot do error detection.
> >
> > > unsigned long index;
> > > char *endptr;
> > >
> > > index = strtoul(child->name + 9, &endptr, 10);
> > > assert(index >= 0 && !*endptr);
> >
> > Still incorrect; you aren't handling errno properly for detecting all
> > errors.  Even better is to use qemu_strtoul(), which already handles
> > proper error detection.
>
> Yeah, I keep forgetting that it returns ULONG_MAX on range error...

Yes, we should limit the range to INT_MAX. How do you like the following 
codes, i just steal it from xen_host_pci_get_value().

int rc;
const char *endptr;
unsigned long value;

assert(!strncmp(child->name, "children.", 9));
rc = qemu_strtoul(child->name + 9, &endptr, 10, &value);
if (!rc) {
    assert(value <= INT_MAX);
    index = value;
} else {
    error_setg_errno(errp, -rc, "Failed to parse value '%s'",
                     child->name + 9);
    return;
}

Thanks
        -Xie

> Max