On 03/30/2010 10:20 AM, Matthias Bolte wrote:
This also fixes a problem with MinGW's GCC on Windows. GCC complained bout the L modifier being unknown.
s/bout/about/
--- src/util/pci.c | 59 +++++++++++++++++++++++++++++++++++++++++++------------ 1 files changed, 46 insertions(+), 13 deletions(-)
diff --git a/src/util/pci.c b/src/util/pci.c index 99ec22a..6aa2fe0 100644 --- a/src/util/pci.c +++ b/src/util/pci.c @@ -282,15 +282,26 @@ pciIterDevices(pciIterPredicate predicate, }
while ((entry = readdir(dir))) { - unsigned domain, bus, slot, function; + unsigned int domain, bus, slot, function; pciDevice *check; + char *tmp;
/* Ignore '.' and '..' */ if (entry->d_name[0] == '.') continue;
- if (sscanf(entry->d_name, "%x:%x:%x.%x", - &domain, &bus, &slot, &function) < 4) { + /* expected format: <domain>:<bus>:<slot>.<function> */ + if (/* domain */ + virStrToLong_ui(entry->d_name, &tmp, 16, &domain) < 0 || + tmp == NULL || *tmp != ':' ||
Useless checks for NULL throughout.
+ /* bus */ + virStrToLong_ui(tmp + 1, &tmp, 16, &bus) < 0 || + tmp == NULL || *tmp != ':' || + /* slot */ + virStrToLong_ui(tmp + 1, &tmp, 16, &slot) < 0 || + tmp == NULL || *tmp != '.' || + /* function */ + virStrToLong_ui(tmp + 1, NULL, 16, &function) < 0) {
Another trailing garbage enforcement worth documenting.
VIR_WARN("Unusual entry in " PCI_SYSFS "devices: %s", entry->d_name); continue; } @@ -914,10 +925,9 @@ pciWaitForDeviceCleanup(pciDevice *dev, const char *matcher) FILE *fp; char line[160]; unsigned long long start, end; - int consumed; char *rest; - unsigned long long domain; - int bus, slot, function; + char *tmp; + unsigned int domain, bus, slot, function;
Why the change in the size of domain?
int in_matching_device; int ret; size_t match_depth; @@ -945,10 +955,18 @@ pciWaitForDeviceCleanup(pciDevice *dev, const char *matcher) * of these situations */ if (in_matching_device && (strspn(line, " ") == (match_depth + 2))) { - if (sscanf(line, "%Lx-%Lx : %n", &start, &end, &consumed) != 2) + tmp = line + strspn(line, " "); + + /* expected format: <start>-<end> : <suffix> */ + if (/* start */ + virStrToLong_ull(tmp, &tmp, 16, &start) < 0 || + tmp == NULL || *tmp != '-' || + /* end */ + virStrToLong_ull(tmp + 1, &tmp, 16, &end) < 0 || + tmp == NULL || ! STRPREFIX(tmp, " : ")) continue;
- rest = line + consumed; + rest = tmp + 3; /* = strlen(" : ") */
Hmm, thinking aloud here: your patch series has introduced several instances of checking STRPREFIX, then advancing to the end of the prefix. Maybe it's time to introduce a helper macro that does both the check and advances the pointer to the end of the match? Something like: #define STPCMP(ptr, cmp) \ (STRPREFIX (*(ptr), cmp) ? (ptr) += strlen (cmp) : NULL) STPCMP(&tmp, " : "); if (tmp == NULL) error... else use tmp... -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org