Re: RFC: introduce common.m4

[Pedro Alves <palves at redhat dot com>]

On 04/24/2013 09:12 PM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
> 
> Pedro> I think you misunderstood the question.
> 
> Indeed.  Sorry about that.
> 
>>> The rule I propose is that if something is needed or used by common,
>>> it should be checked for by common.m4; but that code outside this
>>> directory also be free to use these results.  This means that removing
>>> checks from common.m4 must first be preceded by looking at uses in gdb
>>> and gdbserver.  I think this is pretty easy to do -- easier than what
>>> we are doing now -- and I have documented the requirement.
> 
> Pedro> over keeping common aware of the checks it needs to do (in common.m4),
> Pedro> and gdb/ and gdbserver/ also doing the checks they need for code under
> Pedro> gdb/ and gdbserver/ respectively.
> 
> Pedro> Of course the current status of needing to update gdb and gdbserver in
> Pedro> parallel for common/ things is no good.
> 
> Yeah, there is no deep reason for it other than wanting to avoid
> duplicate checks.  The cache would work fine for the performance aspect.
> I can certainly just drop the configure.ac changes if you think that
> yields a better result.

So, we have:

$ grep "HAVE_LINUX_PERF_EVENT_H\|HAVE_LOCALE_H\|HAVE_MEMORY_H\|HAVE_SIGNAL_H\|HAVE_STRING_H\|HAVE_STRINGS_H\|HAVE_SYS_RESOURCE_H\|HAVE_SYS_UN_H\|HAVE_SYS_WAIT_H\|HAVE_THREAD_DB_H\|HAVE_WAIT_H" gdb/*.[ch] gdb/gdbserver/*.[ch]
gdb/m32c-tdep.c:#if defined (HAVE_STRING_H)
gdb/utils.c:#ifdef HAVE_SYS_RESOURCE_H
gdb/utils.c:#endif /* HAVE_SYS_RESOURCE_H */
gdb/gdbserver/gdbreplay.c:#if HAVE_SIGNAL_H
gdb/gdbserver/gdbreplay.c:#ifdef HAVE_STRING_H
gdb/gdbserver/remote-utils.c:#if HAVE_SIGNAL_H
gdb/gdbserver/server.c:#if HAVE_SIGNAL_H
gdb/gdbserver/server.h:#ifdef HAVE_STRING_H

This actually reveals that m32c-tdep.c, gdbreplay.c and server.h
should be including gdb_string.h (or nothing) instead of signal.h directly.

That'd leave a check for sys/resource.h in gdb, and
a check for signal.h in gdbserver.

$ grep "HAVE_DECL_STRERROR\|HAVE_DECL_STRSTR" gdb/*.[ch] gdb/gdbserver/*.[ch]
gdb/gdbserver/server.h:#if !HAVE_DECL_STRERROR

shows that server.h is doing what gdb_string.h already does
with that HAVE_DECL_STRERROR check, so that bit could be
removed when server.h is made to include gdb_string.h.

And:

$ grep "HAVE_FDWALK\|HAVE_GETRLIMIT\|HAVE_PIPE\|HAVE_PIPE2\|HAVE_SOCKETPAIR" gdb/*.[ch] gdb/gdbserver/*.[ch]
gdb/ser-pipe.c:#if !HAVE_SOCKETPAIR
gdb/ser-pipe.c:#if !HAVE_SOCKETPAIR
gdb/utils.c:#ifdef HAVE_GETRLIMIT
gdb/utils.c:#endif /* HAVE_GETRLIMIT */

Shows we would leave checks for socketpair/getrlimit in gdb/.

IMO, it's a little better if each subdirectory treats the
others more as black boxes.  gdb/ relying on common/'s
HAVE_FOO checks feels like gdb/ relying on common/'s
implementation details to me.  But I don't want to impose.

Thanks,
-- 
Pedro Alves