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