[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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.

-- 
Eric Blake   eblake redhat com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]