[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/2] coroutine-ucontext: Help valgrind understan
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [PATCH 1/2] coroutine-ucontext: Help valgrind understand coroutines |
Date: |
Fri, 13 Jul 2012 10:53:25 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:13.0) Gecko/20120605 Thunderbird/13.0 |
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 <address@hidden>
>>> ---
>>> 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 <valgrind/valgrind.h>
>>> +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
Re: [Qemu-devel] [PATCH 0/2] qemu-iotests: valgrind support, Paolo Bonzini, 2012/07/12