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

[Chris Lalancette]

On 03/09/2010 04:02 PM, Eric Blake wrote:
> 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.

Thanks, I've pushed this now.

-- 
Chris Lalancette