On 02/28/2012 12:58 PM, Daniel P. Berrange wrote:
> On Fri, Feb 17, 2012 at 01:00:22PM +0530, Prerna wrote:
>> The last patch was mangled by my mailer, resending.
>>
>> From: Prerna Saxena <prerna@xxxxxxxxxxxxxxxxxx>
>> Date: Thu, 16 Feb 2012 15:33:43 +0530
>> Subject: [PATCH 2/2] Sysinfo : Allow x86 to fetch sysinfo from /proc/cpuinfo
>> in the event 'dmidecode' is absent in the system.
>>
>> Until now, libvirt on x86 flags an error message if dmidecode is not
>> found. With this patch, the following is a sample output on x86 when
>> dmidecode is absent:
>>
>> ---
>> src/util/sysinfo.c | 95 +++++++++++++++++++++++++++++++++++++++++++++++++--
>> 1 files changed, 91 insertions(+), 4 deletions(-)
>
> ACK
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@xxxxxxxxxx +1-919-301-3266
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