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. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org