[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 01/10] Replace sscanf in legacy device address parsing



On 03/30/2010 10:20 AM, Matthias Bolte wrote:
> +static int
> +virDomainParseLegacyDeviceAddress(char *devaddr,
> +                                  virDomainDevicePCIAddressPtr pci)
> +{
> +    char *tmp = devaddr + strspn(devaddr, " \t\r\n");

Why skip leading whitespace yourself...

> +
> +    /* expected format: <domain>:<bus>:<slot> */
> +    if (virStrToLong_ui(tmp, &tmp, 16, &pci->domain) < 0 ||

when virStrToLong_ui already does the same thing for you?

> +        tmp == NULL || *tmp != ':')

tmp cannot be NULL at this point.

> +        return -1;
> +
> +    if (virStrToLong_ui(tmp + 1, &tmp, 16, &pci->bus) < 0 ||
> +        tmp == NULL || *tmp != ':')

Likewise.

> +        return -1;
> +
> +    if (virStrToLong_ui(tmp + 1, &tmp, 16, &pci->slot) < 0)

Do we care if there is any garbage in *tmp at this point?

> +        return -1;
> +
> +    return 0;
> +}
>  
>  int
>  virDomainDiskDefAssignAddress(virDomainDiskDefPtr def)
> @@ -1541,10 +1561,8 @@ virDomainDiskDefParseXML(xmlNodePtr node,
>      }
>  
>      if (devaddr) {
> -        if (sscanf(devaddr, "%x:%x:%x",
> -                   &def->info.addr.pci.domain,
> -                   &def->info.addr.pci.bus,
> -                   &def->info.addr.pci.slot) < 3) {
> +        if (virDomainParseLegacyDeviceAddress(devaddr,
> +                                              &def->info.addr.pci) < 3) {

Oops - virDomainParseLegacyDeviceAddress returns 0, not 3, on success.
The other two conversions in this patch looked okay.

By the way, eventually your patch series should probably enable
sc_prohibit_atoi_atof in cfg.mk's local-checks-to-skip (if you haven't
already been experimenting with that).  Turning that on will allow 'make
syntax-check' to catch all uses of scanf/atoi.

-- 
Eric Blake   eblake redhat com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]