From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH 0 of 4] [kvm-userspace][test] consolidate test libs to libcflat Date: Sat, 05 Jul 2008 20:36:59 +0300 Message-ID: <486FB13B.5000506@qumranet.com> References: <48675175.9060709@qumranet.com> <48678D03.3010109@qumranet.com> <1214897588.6867.13.camel@thinkpadL> <486F3EB0.6040004@qumranet.com> <1215279153.8921.2.camel@thinkpadL> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Cc: kvm@vger.kernel.org, kvm-ppc@vger.kernel.org, hollisb To: jyoung5@us.ibm.com Return-path: Received: from il.qumranet.com ([212.179.150.194]:13296 "EHLO il.qumranet.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756336AbYGERgm (ORCPT ); Sat, 5 Jul 2008 13:36:42 -0400 In-Reply-To: <1215279153.8921.2.camel@thinkpadL> Sender: kvm-owner@vger.kernel.org List-ID: Jerone Young wrote: > On Sat, 2008-07-05 at 12:28 +0300, Avi Kivity wrote: > >> Jerone Young wrote: >> >>> On Sun, 2008-06-29 at 16:24 +0300, Avi Kivity wrote: >>> >>> >>>> Avi Kivity wrote: >>>> >>>> >>>>> Jerone Young wrote: >>>>> >>>>> >>>>>> This set of patches are to consolidate test libraries into a single >>>>>> library archive. This lib archive is libcflat. This will allow common >>>>>> code to be shared among archs. >>>>>> >>>>>> >>>>>> >>>>>> >>>>> Applied all, thanks. I squashed together 1-3 as Hollis suggested, and >>>>> with git's rename detection, one can see that you merged exit() puts() >>>>> etc into a single file. That usually doesn't work will with libraries >>>>> (if you define your own puts() but not your own exit(), you're in >>>>> trouble), but we can fix this if/when it starts to hurt. >>>>> >>>>> >>>>> >>>>> >>>> This fails compilation with: >>>> >>>> gcc -m64 -D__x86_64__ -I /tmp/BUILDER/kvm/rpmtop/BUILD/kernel/include >>>> -O1 -MMD -MF test/x86/.access.d -g -fomit-frame-pointer -Wa >>>> ll -fno-stack-protector -I ../libkvm -std=gnu99 -ffreestanding -I >>>> test/lib -I test/lib/x86 -c -o test/x86/access.o test/x86/a >>>> ccess.c >>>> test/x86/access.c:153: error: conflicting types for 'memset' >>>> /usr/include/string.h:59: error: previous declaration of 'memset' was here >>>> test/x86/access.c: In function 'ac_test_do_access': >>>> test/x86/access.c:511: warning: implicit declaration of function 'printf' >>>> test/x86/access.c: In function 'main': >>>> test/x86/access.c:578: warning: passing argument 1 of 'smp_init' from >>>> incompatible pointer type >>>> make[1]: *** [test/x86/access.o] Error 1 >>>> make[1]: Leaving directory `/tmp/BUILDER/kvm/rpmtop/BUILD/user' >>>> error: Bad exit status from /var/tmp/rpm-tmp.45450 (%build) >>>> >>>> So I'm unapplying these patches. >>>> >>>> >>>> >>> Ah. Sorry about that. I was testing on x86 and not x86-64. So I didn't >>> notice that config-x86_64.mak had other test cases outside of what was >>> in config-x86-common. >>> >>> The problem here is access.c needs to include the new libcflat.h , and >>> not printf.h . In one of my patches. >>> >>> Since I don't currently have an x86-64 setup to try, can't turn out a >>> patch. I can give you an idea of what is up so here are the changes >>> needed for test/x86/access.c: >>> >>> jerone@thinkpadL:~/work/kvm-userspace/user/test/x86$ vi access.c.FIXED >>> jerone@thinkpadL:~/work/kvm-userspace/user/test/x86$ diff -aurp access.c access.c.FIXED >>> --- access.c 2008-06-09 08:57:25.000000000 -0500 >>> +++ access.c.FIXED 2008-07-01 02:32:41.000000000 -0500 >>> @@ -1,7 +1,6 @@ >>> >>> -#include "smp.h" >>> -#include "printf.h" >>> -#include "string.h" >>> +#include >>> +#include >>> >>> >>> >> Er, <> includes are for system headers. Please stick to "" includes. >> The previous patches also need to be fixed. >> > > Actually the include paths to not use system includes. > > <> means system include file. libcflat.h is not a system include file, so no pointy brackets. > Just setup an x86-64 compilation enviroment. Looks like access.c was the > only test that had the problem. Also the pseudo patch I sent ended up > being the solution. I will send a formal patch set shortly so you can > include libcflat. > Thanks. Please don't send separate create and delete patches, and use diff -M, so that we see exactly what's changed rather than a zillion and a half adds and removes. -- Do not meddle in the internals of kernels, for they are subtle and quick to panic.