Re: [patch+NEWS] Avoid false valgrind warnings on linux_ptrace_test_ret_to_nx

Pedro Alves
2013-02-20T15:56:23Z

On 02/20/2013 03:23 PM, Jan Kratochvil wrote:

>> I assumed there'd be a reason for the more complete test,
>> instead of a simpler and unconditional
>> AC_CHECK_HEADERS(valgrind/valgrind.h) and then just
>>
>> #ifdef RUNNING_ON_VALGRIND
>> if (RUNNING_ON_VALGRIND)
>> return;
>> #endif
>>
>> As in, you had found that older valgrind/valgrind.h's didn't
>> have that RUNNING_ON_VALGRIND macro.
>
> I haven't found such valgrind. The reason is there could be a broken
> valgrind.h include file installed. As GDB built before not depending on such
> file in the past and now it would break it would be a regression.
>
> Or is somewhere a rule what should and what should not be tested by configure?

Hmm. I don't see how I can reply to that with a one liner, so
apologies in advance for the long text.

The basic configure "rule" is test for features you need.
There's no way to be more strict and more than guidance. Case by
case judgment needs to be applied.

However, one unwritten rule (not just GDB's) that applies more broadly
than configure things is to not overengineer things, do the minimal
necessary, and worry only about unknown problems if they turn
out to be real problems. The applied example of that being the
general rule would be that we don't have autoconf checks for every
function and macro we use from all the headers we include. From that
perspective, the simpler approach would be better, as we'd have
less code to maintain, and fewer configure knobs to document
and explain to users.

To break the build, either the header would be completely broken, and
would break every program that includes it (unlikely, used by several
programs in a distro), or, RUNNING_ON_VALGRIND would be broken (unlikely,
likewise), or the header would use some symbol that would conflict with
the other headers the linux-ptrace.c file includes before valgrind.h, which
is possible, but is not something that is checked by the new
autoconf check, so the most likely (IMO) scenario would be broken
anyway.

That said, I really don't intend to argue.

--
Pedro Alves