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

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1105 bytes --]

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.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]