[ofa-general] [PATCH] librdmacm 1/2: add valgrind support to auto-tools configuration file

[Dotan Barak]

Sean Hefty wrote:
>>   The resulting executables will still run without Valgrind, just a
>>   little bit more slowly than they otherwise would, but otherwise
>>   unchanged.  When not running on valgrind, each client request
>>   consumes very few (eg. 7) instructions, so the resulting performance
>>   loss is negligible unless you plan to execute client requests
>>   millions of times per second.  Nevertheless, if that is still a
>>   problem, you can compile with the NVALGRIND symbol defined (gcc
>>   -DNVALGRIND) so that client requests are not even compiled in.  */
>>     
>
> It just seems that we're using two checks where we really want one.  We check to
> see if memcheck.h exists, and if it does, we include it.  But if the user
> doesn't want valgrind, then we disable using it through NVALGRIND.
>
> How about something more like:
>
> if with-valgrind is set then {
> 	if memcheck is found then
> 		INCLUDE_MEMCHECK_H = 1
> 		if with_valgrind then set CPPFLAGS
> 	else
> 		error
> }
>
> The error message in your second patch is a little misleading otherwise, since
> it could occur even though valgrind support was not requested.
>
> - Sean
>
>   
Basically, you are right.
I wrote this piece of code based on the libibverbs to be consistent with 
it, maybe Roland can contribute to this thread too ...

I think that the way it is written now, it allows to the library to be 
prepared to valgrind anyway and use its header file when possible
(to allow the user later on use valgrind) unless the user explicitly 
don't want to use valgrind (to avoid the minor overhead of marking
the buffer as valid) or if the user explicitly specified that he wants 
valgrind, but the requested header file doesn't exist.

I think that we should keep this as the way i suggested because 
libibverbs work this way for quite a while and there wasn't any problem
with this implementation and i would like to be consistent with it.

what do you think?

thanks
Dotan