Re: [Qemu-devel] [PATCH] qdev-properties: Fix (u)intXX parsers

[Kevin Wolf <kwolf@redhat.com>]

Am 26.05.2010 12:45, schrieb Daniel P. Berrange:
> On Wed, May 26, 2010 at 12:28:13PM +0200, Kevin Wolf wrote:
>> scanf calls must not use PRI constants, they have probably the wrong size and
>> corrupt memory. We could replace them by SCN ones, but strtol is simpler than
>> scanf here anyway. While at it, also fix the parsers to reject garbage after
>> the number ("4096xyz" was accepted before).
>>
>> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
>> ---
>>  hw/qdev-properties.c |   50 +++++++++++++++++++++++++++++++++++---------------
>>  1 files changed, 35 insertions(+), 15 deletions(-)
>>
>> diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
>> index 9ffdba7..9a61ca2 100644
>> --- a/hw/qdev-properties.c
>> +++ b/hw/qdev-properties.c
>> @@ -68,12 +68,14 @@ PropertyInfo qdev_prop_bit = {
>>  static int parse_uint8(DeviceState *dev, Property *prop, const char *str)
>>  {
>>      uint8_t *ptr = qdev_get_prop_ptr(dev, prop);
>> -    const char *fmt;
>> +    char *end;
>>  
>>      /* accept both hex and decimal */
>> -    fmt = strncasecmp(str, "0x",2) == 0 ? "%" PRIx8 : "%" PRIu8;
>> -    if (sscanf(str, fmt, ptr) != 1)
>> +    *ptr = strtoul(str, &end, 0);
>> +    if (end != str + strlen(str)) {
>>          return -EINVAL;
>> +    }
> 
> I think you can avoid the O(n) operation here & in the other cases with
> a test like this:
> 
>     if ((end == str) || (*end != '\0'))
>        return -EINVAL

It probably doesn't really make a difference here, but you're right.
I'll send another version with this change.

Kevin