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
_______________________________________________
general mailing list
[email protected]
http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general
To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
