From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:32875) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Spbd2-0001a2-Mc for qemu-devel@nongnu.org; Fri, 13 Jul 2012 04:53:42 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Spbcu-00021J-MZ for qemu-devel@nongnu.org; Fri, 13 Jul 2012 04:53:36 -0400 Received: from mx1.redhat.com ([209.132.183.28]:16698) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Spbcu-000218-DK for qemu-devel@nongnu.org; Fri, 13 Jul 2012 04:53:28 -0400 Message-ID: <4FFFE205.7000909@redhat.com> Date: Fri, 13 Jul 2012 10:53:25 +0200 From: Kevin Wolf MIME-Version: 1.0 References: <1342103249-20888-1-git-send-email-kwolf@redhat.com> <1342103249-20888-2-git-send-email-kwolf@redhat.com> <4FFF0446.7000009@weilnetz.de> <4FFFE043.4050604@redhat.com> In-Reply-To: <4FFFE043.4050604@redhat.com> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 1/2] coroutine-ucontext: Help valgrind understand coroutines List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Weil Cc: qemu-devel@nongnu.org Am 13.07.2012 10:45, schrieb Kevin Wolf: > Am 12.07.2012 19:07, schrieb Stefan Weil: >> Great that you address this issue! >> I have two annotations, please see below. >> >> >> Am 12.07.2012 16:27, schrieb Kevin Wolf: >>> valgrind tends to get confused and report false positives when you >>> switch stacks and don't tell it about it. >>> >>> Signed-off-by: Kevin Wolf >>> --- >>> configure | 18 ++++++++++++++++++ >>> coroutine-ucontext.c | 21 +++++++++++++++++++++ >>> 2 files changed, 39 insertions(+), 0 deletions(-) >>> >>> diff --git a/configure b/configure >>> index 500fe24..b424fcf 100755 >>> --- a/configure >>> +++ b/configure >>> @@ -2855,6 +2855,20 @@ if compile_prog "" "" ; then >>> fi >>> >>> ######################################## >>> +# check if we have valgrind/valgrind.h >>> + >>> +valgrind_h=no >>> +cat > $TMPC << EOF >>> +#include >>> +int main(void) { >>> + return 0; >>> +} >>> +EOF >>> +if compile_prog "" "" ; then >>> + valgrind_h=yes >>> +fi >>> + >>> +######################################## >>> # check if environ is declared >>> >>> has_environ=no >>> @@ -3380,6 +3394,10 @@ if test "$linux_magic_h" = "yes" ; then >>> echo "CONFIG_LINUX_MAGIC_H=y" >> $config_host_mak >>> fi >>> >>> +if test "$valgrind_h" = "yes" ; then >>> + echo "CONFIG_VALGRIND_H=y" >> $config_host_mak >> >> I'd prefer CONFIG_VALGRIND instead of CONFIG_VALGRIND_H. >> The important feature is Valgrind, not the valgrind.h which is >> needed to get that feature. >> >> Of course that is a matter of personal taste, and there are >> already a few CONFIG_SOMETHING_H macros, but most >> macros omit the _H even if there _is_ a related h file. > > Okay, I don't really mind, it was just the style of the check > immediately before the new one, so I used that. I can change it. Why did you #define a CONFIG_VALGRIND locally in commit c2a8238a? I always thought, CONFIG_* macros should only ever be defined by configure. Now this gives me a naming conflict with something that depends on different semantics. I'll keep CONFIG_VALGRIND_H and leave it to you to clean up oslib-posix.c if you care enough. Kevin