Re: [libvirt] [Resending] [PATCH 2/2] Allow x86 to fetch sysinfo from /proc/cpuinfo when dmidecode is absent.

On 02/28/2012 12:58 PM, Daniel P. Berrange wrote: NACK; we need a v2 for both patches. You had TABs (run 'make syntax-check'), and this compiler warning is evidence of a real bug: util/sysinfo.c: In function 'virSysinfoParseCPUInfoProcessor': util/sysinfo.c:622:9: error: passing argument 1 of 'virSkipSpaces' from incompatible pointer type [-Werror] util/util.h:162:6: note: expected 'const char **' but argument is of type 'char **' Looking at that code: if ((eol) && ((processor->processor_socket_destination = strndup(cur, eol - cur)) == NULL)) goto no_memory; virSkipSpaces(&(processor->processor_socket_destination)); but virSkipSpaces is _designed_ to advance the pointer. Which means that when you later call VIR_FREE on the altered pointer, you aren't freeing the pointer returned by the malloc inside strndup, but are instead freeing some random address occurring later in the allocated chunk of memory - a surefire way to corrupt your heap. I didn't compile-test the PowerPC code, but see the same bug there by inspection. If you want to skip spaces, you need to skip them in 'cur', prior to calling strndup; something like: diff --git i/src/util/sysinfo.c w/src/util/sysinfo.c index de78d23..f8095e5 100644 --- i/src/util/sysinfo.c +++ w/src/util/sysinfo.c @@ -602,13 +602,15 @@ no_memory: static int virSysinfoParseCPUInfoProcessor(const char *base, virSysinfoDefPtr ret) { - char *cur, *eol, *tmp_base; + const char *cur; + char *eol, *tmp_base; virSysinfoProcessorDefPtr processor; while((tmp_base = strstr(base, "processor")) != NULL) { base = tmp_base; eol = strchr(base, '\n'); cur = strchr(base, ':') + 1; + virSkipSpaces(&cur); if (VIR_EXPAND_N(ret->processor, ret->nprocessor, 1) < 0) { goto no_memory; @@ -619,7 +621,6 @@ virSysinfoParseCPUInfoProcessor(const char *base, virSysinfoDefPtr ret) ((processor->processor_socket_destination = strndup(cur, eol - cur)) == NULL)) goto no_memory; - virSkipSpaces(&(processor->processor_socket_destination)); if ((cur = strstr(base, "vendor_id")) != NULL) { cur = strchr(cur, ':') + 1; -- Eric Blake eblake(a)redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org