Re: [libvirt] [PATCH] Fix up nodeinfo parsing code.

On 03/09/2010 12:30 PM, Chris Lalancette wrote: > 1) Do more work at compile time instead of runtime (minor) > 2) Properly handle the hex digits that come from > /sys/devices/system/cpu/cpu*/topology/thread_siblings > 3) Fix up some error paths that could cause SEGV > 4) Used unsigned's for the cpu numbers (cpu -1 doesn't > make any sense) Kudos for catching some that I missed. > -static int parse_socket(int cpu) > +static int parse_socket(unsigned int cpu) > { > - char *path = NULL; > + char *path; > FILE *pathfp; > char socket_str[1024]; > char *tmp; > - int socket; > + int socket = -1; > > - if (virAsprintf(&path, "%s/cpu%d/topology/physical_package_id", > - CPU_SYS_PATH, cpu) < 0) { > + if (virAsprintf(&path, CPU_SYS_PATH "/cpu%u/topology/physical_package_id", > + cpu) < 0) { > virReportOOMError(); > return -1; > } > @@ -120,7 +124,8 @@ static int parse_socket(int cpu) > pathfp = fopen(path, "r"); > if (pathfp == NULL) { > virReportSystemError(errno, _("cannot open %s"), path); > - goto cleanup; > + VIR_FREE(path); > + return -1; > } Definitely some SEGV avoidance factors. > @@ -244,6 +249,18 @@ int linuxNodeInfoCPUPopulate(virConnectPtr conn, FILE *cpuinfo, > > closedir(cpudir); > > + /* there should always be at least one socket and one thread */ > + if (nodeinfo->sockets == 0) { > + nodeReportError(conn, VIR_ERR_INTERNAL_ERROR, > + "%s", _("no sockets found")); > + return -1; > + } > + if (nodeinfo->threads == 0) { > + nodeReportError(conn, VIR_ERR_INTERNAL_ERROR, > + "%s", _("no threads found")); > + return -1; > + } I didn't see this mentioned in the commit log, but it's a good change. Overall: ACK.