Re: [Qemu-devel] [PATCH v4 1/5] Use qemu_strtoul instead of strtol

[Cao jin]

On 01/09/2016 01:35 AM, Eric Blake wrote:
> On 01/08/2016 01:37 AM, Cao jin wrote:
> > strtol() don`t guarantee errno to be ERANGE on overflow.
>
> I stand slightly corrected: C99 requires ERANGE on overflow, but not
> EINVAL; it is POSIX that adds EINVAL, but does not properly require it.
>   At any rate, my main point was that errno is not always properly set by
> all strtol implementations, and furthermore that you can't rely on it
> being set to a sane value if you didn't pre-set it to 0.

Got you:)

> > This wrapper returns either -EINVAL or the errno set by strtol()
> > function (e.g -ERANGE).
>
> The subject line doesn't start with a topic.  Maybe a better commit
> message would be:
>
> xen: Use qemu_strtoul instead of strtol
>
> No need to roll our own (with slightly incorrect handling of errno),
> when we can use the common version.

ok, looks good.

> > Signed-off-by: Cao jin <address@hidden>
> > ---
> >   hw/xen/xen-host-pci-device.c | 11 +++--------
> >   1 file changed, 3 insertions(+), 8 deletions(-)
>
> >       buf[rc] = 0;
> > -    value = strtol(buf, &endptr, base);
> > -    if (endptr == buf || *endptr != '\n') {
> > -        rc = -1;
> > -    } else if ((value == LONG_MIN || value == LONG_MAX) && errno == ERANGE) {
> > -        rc = -errno;
> > -    } else {
> > -        rc = 0;
> > +    rc = qemu_strtoul(buf, &endptr, base, &value);
>
> Why did you switch from strtol() to qemu_strtoul()?  Was signed parsing
> incorrect, and unsigned parsing a bug fix?  If so, please mention it in
> the commit message as intentional.  Otherwise, use qemu_strtol() (and
> adjust the commit message accordingly).

Yes, I should mention it, sorry for forgetting to explain it. Because 
what it want to read from host are values in pci device config space, 
which are non-negative, like "vendor ID", "device ID", "class code", 
etc. so unsigned parsing seems more suitable in this case. we can tell 
it also from the local variable "unsigned long value;"

--
Yours Sincerely,

Cao jin