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 [email protected] +1-801-349-2682
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list [email protected] https://www.redhat.com/mailman/listinfo/libvir-list
