Re: [Qemu-devel] [PATCH v5 2/5] Add Error **errp for xen_host_pci_device_get()

[Cao jin]

On 01/16/2016 12:41 AM, Eric Blake wrote:
> On 01/14/2016 08:11 PM, Cao jin wrote:
>
> > > >        buf[rc] = 0;
> > > > -    rc = qemu_strtoul(buf, &endptr, base, &value);
> > > > -    if (!rc) {
> > > > -        *pvalue = value;
> > > > +    rc = qemu_strtoul(buf, &endptr, base, (unsigned long *)pvalue);
> > >
> > > Ouch. Casting unsigned int * to unsigned long * and then dereferencing
> > > it is bogus (you end up having qemu_strtoul() write beyond bounds on
> > > platforms where long is larger than int).
> >
> > Yes, I considered this issue a little. Because the current condition is:
> > the value it want to get won`t exceed 4 byte (vendor/device ID, etc). So
> > I guess even if on x86_64(length of int != long), it won`t break things.
> > So, compared with following, which style do you prefer?
>
> Maybe:
>
> rc = qemu_strtoul(buf, &endptr, base, &value);
> if (rc) {
>      assert(value < UINT_MAX);
>      *pvalue = value;
> } else {
>      report error ...
> }
>
> And maybe some of it should even be done as part of the conversion to
> qemu_strtoul() in 1/5.

Thanks for the example, will give v6 soon.
--
Yours Sincerely,

Cao jin