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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list