Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [RFA]: Remove unused header files.
@ 2001-03-01 17:44 J.T. Conklin
  2001-03-02  8:25 ` Andrew Cagney
                   ` (5 more replies)
  0 siblings, 6 replies; 18+ messages in thread
From: J.T. Conklin @ 2001-03-01 17:44 UTC (permalink / raw)
  To: gdb-patches

The enclosed patch removes some more unused header files from *.c
files.  Unlike the last time, this list was generated from a lint
program that ensures that no macro, typedef, struct, union, enum, or
declaration was used from the header.  This was run on one config, so
there are many files that haven't been processed yet.  First things
first.

In several instances it identified two header files, <errno.h> and
<linespec.h>, as unnecessary because <errno.h> is unconditionally
included in defs.h; and the only declaration in linespec.h
(decode_line_1) is also defined in symtab.h.

I'm unsure about the correct way to handle those are.  As long as
errno.h is included in defs.h, I see no reason to include it in any
*.c file.  One could argue that errno.h should be remove from defs.h,
but since errno (or the E* macros) is used in almost every file, I 
suspect we'd have to #include it in many files.

The decode_line_1() declaration (IMO) indicates a unclear separation
of interface and implementation.  (Again, IMO) a function declaration
should only be in one header.  Please share your thoughts, on this 
specific case, and in general.  decode_line_1() is the first of many
functions that have multiple declarations.

2001-03-01  J.T. Conklin  <jtc@redback.com>

	* arch-utils.c (#include <ctype.h>): Removed.
	* c-typeprint.c: Likewise.
	* dbxread.c: Likewise.
	* gdbtypes.c: Likewise.

	* c-valprint.c (#include "demangle.h"): Removed.
	* ch-typeprint.c: Likewise.
	* eval.c: Likewise.
	* f-typeprint.c: Likewise.
	* f-valprint.c: Likewise.
	* m2-typeprint.c: Likewise.
	* p-typeprint.c: Likewise.

	* m2-typeprint.c (#include "gdb_string.h"): Removed.
	* nlmread.c: Likewise.

	* minsyms.c (#include "gdb-stabs.h"): Removed.
	* mipsread.c: Likewise.
	* nlmread.c: Likewise.

	* m2-typeprint.c (#include "obstack.h"): Removed.
	* m2-valprint.c: Likewise.

	* event-loop.c (#include <setjmp.h>): Removed.

Index: arch-utils.c
===================================================================
RCS file: /cvs/src/src/gdb/arch-utils.c,v
retrieving revision 1.20
diff -c -r1.20 arch-utils.c
*** arch-utils.c	2001/02/08 06:03:52	1.20
--- arch-utils.c	2001/03/02 01:24:58
***************
*** 27,33 ****
  /* Just include everything in sight so that the every old definition
     of macro is visible. */
  #include "gdb_string.h"
- #include <ctype.h>
  #include "symtab.h"
  #include "frame.h"
  #include "inferior.h"
--- 27,32 ----
Index: c-typeprint.c
===================================================================
RCS file: /cvs/src/src/gdb/c-typeprint.c,v
retrieving revision 1.4
diff -c -r1.4 c-typeprint.c
*** c-typeprint.c	2000/12/15 01:01:46	1.4
--- c-typeprint.c	2001/03/02 01:25:12
***************
*** 37,43 ****
  
  #include "gdb_string.h"
  #include <errno.h>
- #include <ctype.h>
  
  /* Flag indicating target was compiled by HP compiler */
  extern int hp_som_som_object_present;
--- 37,42 ----
Index: c-valprint.c
===================================================================
RCS file: /cvs/src/src/gdb/c-valprint.c,v
retrieving revision 1.6
diff -c -r1.6 c-valprint.c
*** c-valprint.c	2000/11/20 20:33:53	1.6
--- c-valprint.c	2001/03/02 01:25:13
***************
*** 24,30 ****
  #include "gdbtypes.h"
  #include "expression.h"
  #include "value.h"
- #include "demangle.h"
  #include "valprint.h"
  #include "language.h"
  #include "c-lang.h"
--- 24,29 ----
Index: ch-typeprint.c
===================================================================
RCS file: /cvs/src/src/gdb/ch-typeprint.c,v
retrieving revision 1.2
diff -c -r1.2 ch-typeprint.c
*** ch-typeprint.c	2000/07/30 01:48:24	1.2
--- ch-typeprint.c	2001/03/02 01:25:15
***************
*** 30,36 ****
  #include "command.h"
  #include "gdbcmd.h"
  #include "language.h"
- #include "demangle.h"
  #include "ch-lang.h"
  #include "typeprint.h"
  
--- 30,35 ----
Index: dbxread.c
===================================================================
RCS file: /cvs/src/src/gdb/dbxread.c,v
retrieving revision 1.13
diff -c -r1.13 dbxread.c
*** dbxread.c	2001/02/25 04:45:11	1.13
--- dbxread.c	2001/03/02 01:25:35
***************
*** 43,49 ****
  
  #include "obstack.h"
  #include "gdb_stat.h"
- #include <ctype.h>
  #include "symtab.h"
  #include "breakpoint.h"
  #include "command.h"
--- 43,48 ----
Index: eval.c
===================================================================
RCS file: /cvs/src/src/gdb/eval.c,v
retrieving revision 1.9
diff -c -r1.9 eval.c
*** eval.c	2000/10/30 15:32:51	1.9
--- eval.c	2001/03/02 01:25:39
***************
*** 27,33 ****
  #include "expression.h"
  #include "target.h"
  #include "frame.h"
- #include "demangle.h"
  #include "language.h"		/* For CAST_IS_CONVERSION */
  #include "f-lang.h"		/* for array bound stuff */
  
--- 27,32 ----
Index: event-loop.c
===================================================================
RCS file: /cvs/src/src/gdb/event-loop.c,v
retrieving revision 1.11
diff -c -r1.11 event-loop.c
*** event-loop.c	2001/02/08 06:03:52	1.11
--- event-loop.c	2001/03/02 01:25:41
***************
*** 35,41 ****
  #include <sys/types.h>
  #include "gdb_string.h"
  #include <errno.h>
- #include <setjmp.h>
  #include <sys/time.h>
  
  /* Type of the mask arguments to select. */
--- 35,40 ----
Index: f-typeprint.c
===================================================================
RCS file: /cvs/src/src/gdb/f-typeprint.c,v
retrieving revision 1.2
diff -c -r1.2 f-typeprint.c
*** f-typeprint.c	2000/07/30 01:48:25	1.2
--- f-typeprint.c	2001/03/02 01:25:42
***************
*** 32,38 ****
  #include "command.h"
  #include "gdbcmd.h"
  #include "language.h"
- #include "demangle.h"
  #include "f-lang.h"
  #include "typeprint.h"
  #include "frame.h"		/* ??? */
--- 32,37 ----
Index: f-valprint.c
===================================================================
RCS file: /cvs/src/src/gdb/f-valprint.c,v
retrieving revision 1.4
diff -c -r1.4 f-valprint.c
*** f-valprint.c	2000/07/30 01:48:25	1.4
--- f-valprint.c	2001/03/02 01:25:43
***************
*** 26,32 ****
  #include "gdbtypes.h"
  #include "expression.h"
  #include "value.h"
- #include "demangle.h"
  #include "valprint.h"
  #include "language.h"
  #include "f-lang.h"
--- 26,31 ----
Index: gdbtypes.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbtypes.c,v
retrieving revision 1.16
diff -c -r1.16 gdbtypes.c
*** gdbtypes.c	2000/12/15 01:01:47	1.16
--- gdbtypes.c	2001/03/02 01:25:50
***************
*** 1164,1170 ****
  }
  
  /* New code added to support parsing of Cfront stabs strings */
- #include <ctype.h>
  #define INIT_EXTRA { pextras->len=0; pextras->str[0]='\0'; }
  #define ADD_EXTRA(c) { pextras->str[pextras->len++]=c; }
  
--- 1164,1169 ----
Index: m2-typeprint.c
===================================================================
RCS file: /cvs/src/src/gdb/m2-typeprint.c,v
retrieving revision 1.2
diff -c -r1.2 m2-typeprint.c
*** m2-typeprint.c	2000/07/30 01:48:26	1.2
--- m2-typeprint.c	2001/03/02 01:25:50
***************
*** 19,25 ****
     Boston, MA 02111-1307, USA.  */
  
  #include "defs.h"
- #include "obstack.h"
  #include "bfd.h"		/* Binary File Description */
  #include "symtab.h"
  #include "gdbtypes.h"
--- 19,24 ----
***************
*** 30,39 ****
  #include "command.h"
  #include "gdbcmd.h"
  #include "language.h"
- #include "demangle.h"
  #include "m2-lang.h"
- 
- #include "gdb_string.h"
  #include <errno.h>
  
  void
--- 29,35 ----
Index: m2-valprint.c
===================================================================
RCS file: /cvs/src/src/gdb/m2-valprint.c,v
retrieving revision 1.2
diff -c -r1.2 m2-valprint.c
*** m2-valprint.c	2000/07/30 01:48:26	1.2
--- m2-valprint.c	2001/03/02 01:25:50
***************
*** 19,25 ****
     Boston, MA 02111-1307, USA.  */
  
  #include "defs.h"
- #include "obstack.h"
  #include "symtab.h"
  #include "gdbtypes.h"
  #include "valprint.h"
--- 19,24 ----
Index: minsyms.c
===================================================================
RCS file: /cvs/src/src/gdb/minsyms.c,v
retrieving revision 1.13
diff -c -r1.13 minsyms.c
*** minsyms.c	2001/02/11 06:11:38	1.13
--- minsyms.c	2001/03/02 01:25:52
***************
*** 46,52 ****
  #include "symfile.h"
  #include "objfiles.h"
  #include "demangle.h"
- #include "gdb-stabs.h"
  
  /* Accumulate the minimal symbols for each objfile in bunches of BUNCH_SIZE.
     At the end, copy them all into one newly allocated location on an objfile's
--- 46,51 ----
Index: mipsread.c
===================================================================
RCS file: /cvs/src/src/gdb/mipsread.c,v
retrieving revision 1.6
diff -c -r1.6 mipsread.c
*** mipsread.c	2001/02/10 11:12:06	1.6
--- mipsread.c	2001/03/02 01:25:52
***************
*** 32,38 ****
  #include "objfiles.h"
  #include "buildsym.h"
  #include "stabsread.h"
- #include "gdb-stabs.h"
  
  #include "coff/sym.h"
  #include "coff/internal.h"
--- 32,37 ----
Index: nlmread.c
===================================================================
RCS file: /cvs/src/src/gdb/nlmread.c,v
retrieving revision 1.5
diff -c -r1.5 nlmread.c
*** nlmread.c	2000/12/15 01:01:48	1.5
--- nlmread.c	2001/03/02 01:25:52
***************
*** 20,31 ****
     Boston, MA 02111-1307, USA.  */
  
  #include "defs.h"
- #include "gdb_string.h"
  #include "bfd.h"
  #include "symtab.h"
  #include "symfile.h"
  #include "objfiles.h"
- #include "gdb-stabs.h"
  #include "buildsym.h"
  #include "stabsread.h"
  
--- 20,29 ----
Index: p-typeprint.c
===================================================================
RCS file: /cvs/src/src/gdb/p-typeprint.c,v
retrieving revision 1.3
diff -c -r1.3 p-typeprint.c
*** p-typeprint.c	2000/07/30 01:48:26	1.3
--- p-typeprint.c	2001/03/02 01:25:53
***************
*** 32,38 ****
  #include "command.h"
  #include "gdbcmd.h"
  #include "language.h"
- #include "demangle.h"
  #include "p-lang.h"
  #include "typeprint.h"
  
--- 32,37 ----



-- 
J.T. Conklin
RedBack Networks


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFA]: Remove unused header files.
  2001-03-01 17:44 [RFA]: Remove unused header files J.T. Conklin
@ 2001-03-02  8:25 ` Andrew Cagney
  2001-03-02 16:34   ` J.T. Conklin
  2001-03-05  8:11 ` Andrew Cagney
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 18+ messages in thread
From: Andrew Cagney @ 2001-03-02  8:25 UTC (permalink / raw)
  To: jtc; +Cc: gdb-patches

J.T.

GDB currently has some really obscure and effectivly hidden include
dependencies.  For instance, given the target mn10200-elf you find:

	values.c uses EXTRACT_RETURN_VALUE

	config/mn10200/tm-mn10200.h's definition
		refers to something declared in regcache.h

	values.c includes "tm.h" to get at EXTRACT_RETURN_VALUE and
consequently needs a declaration for regcache.h.

Does your lint handle this?

	Andrew


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFA]: Remove unused header files.
  2001-03-02  8:25 ` Andrew Cagney
@ 2001-03-02 16:34   ` J.T. Conklin
  2001-03-05  8:11     ` Andrew Cagney
  0 siblings, 1 reply; 18+ messages in thread
From: J.T. Conklin @ 2001-03-02 16:34 UTC (permalink / raw)
  To: Andrew Cagney; +Cc: gdb-patches

>>>>> " Andrew" == Andrew Cagney <ac131313@cygnus.com> writes:
Andrew> GDB currently has some really obscure and effectivly hidden include
Andrew> dependencies.  For instance, given the target mn10200-elf you find:
Andrew>
Andrew> 	values.c uses EXTRACT_RETURN_VALUE
Andrew>
Andrew> 	config/mn10200/tm-mn10200.h's definition
Andrew> 		refers to something declared in regcache.h
Andrew>
Andrew> values.c includes "tm.h" to get at EXTRACT_RETURN_VALUE and
Andrew> consequently needs a declaration for regcache.h.
Andrew>
Andrew> Does your lint handle this?

Yes and no.  If I was linting GDB configured for the mn10200, it
regcache.h would be considered necessary, but for other cases it
would not.  Since I'm not planning on linting every possible config
before deciding whether a header is unnecessary, it's not up to the
job.  

However, I question whether it has to be.  Why should GDB have obscure
and effectively hidden include dependencies?  We can eliminate them so
that *.c files include the headers that are required by all configs,
and have tm-, nm-, or xm- headers that require extra headers due to
macro definitions to include them themselves.  Alternatively, we can
include all the headers in defs.h to ensure that all such headers have
access to the declarations.  I'm not fond of the latter --- I'm trying
to decrease the number of headers being processed for each module, not
increase them.

I think requiring the tm-, nm-, and xm- headers to include dependent
headers is desirable.  While it will add some compilation overhead to
some configs, it should be only until they are multi-arched.  That can
be addressed even before multi-arch, since the macros can be converted
to functions and moved to the appropriate -tdep.c or -nat.c file.

        --jtc

-- 
J.T. Conklin
RedBack Networks


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFA]: Remove unused header files.
  2001-03-02 16:34   ` J.T. Conklin
@ 2001-03-05  8:11     ` Andrew Cagney
  2001-03-06 13:08       ` J.T. Conklin
  0 siblings, 1 reply; 18+ messages in thread
From: Andrew Cagney @ 2001-03-05  8:11 UTC (permalink / raw)
  To: jtc; +Cc: gdb-patches

> Yes and no.  If I was linting GDB configured for the mn10200, it
> regcache.h would be considered necessary, but for other cases it
> would not.  Since I'm not planning on linting every possible config
> before deciding whether a header is unnecessary, it's not up to the
> job.

I think we need to clarify one of the ground rules - should "tm.h"
et.al. files be sucking in the things they refer to and (assuming that
is agreed) should that be addressed before include headers are
eliminated from .c files.

If that isn't done, I've the (rational) fear that these cleanups will
start breaking an unreasonable number of existing targets.  Rational?
Well I'm still cleaning up fallout from regcache.h - I've ~50 tm.h files
that should include regcache.h.

	Andrew


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFA]: Remove unused header files.
  2001-03-01 17:44 [RFA]: Remove unused header files J.T. Conklin
                   ` (4 preceding siblings ...)
  2001-03-05  8:11 ` Andrew Cagney
@ 2001-03-05  8:11 ` Andrew Cagney
  2001-03-06  9:20   ` Fernando Nasser
  2001-03-06 13:27   ` J.T. Conklin
  5 siblings, 2 replies; 18+ messages in thread
From: Andrew Cagney @ 2001-03-05  8:11 UTC (permalink / raw)
  To: jtc; +Cc: gdb-patches

> The decode_line_1() declaration (IMO) indicates a unclear separation
> of interface and implementation.  (Again, IMO) a function declaration
> should only be in one header.  Please share your thoughts, on this
> specific case, and in general.  decode_line_1() is the first of many
> functions that have multiple declarations.

Only one? Just don't get me started on the number of extern declarations
that appear in .c files! :-)  Yes, I agree that function declarations
should only appear once.

As for the more general question of confused interfaces.  I think GDB's
internal structure is facing major change (multi-arch).  Consequently
I'm hopeing that that that that work will address many of those
problems.  I suspect that, at this stage, you might want to put blinkers
and just concentrate on the basic #include mess.  That, by its self, is
pretty daunting.

Good luck!
	Andrew


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFA]: Remove unused header files.
  2001-03-01 17:44 [RFA]: Remove unused header files J.T. Conklin
  2001-03-02  8:25 ` Andrew Cagney
  2001-03-05  8:11 ` Andrew Cagney
@ 2001-03-05  8:11 ` Andrew Cagney
  2001-03-06 13:02   ` J.T. Conklin
  2001-03-05  8:11 ` Andrew Cagney
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 18+ messages in thread
From: Andrew Cagney @ 2001-03-05  8:11 UTC (permalink / raw)
  To: jtc; +Cc: gdb-patches

"J.T. Conklin" wrote:

> In several instances it identified two header files, <errno.h> and
> <linespec.h>, as unnecessary because <errno.h> is unconditionally
> included in defs.h; and the only declaration in linespec.h
> (decode_line_1) is also defined in symtab.h.
> 
> I'm unsure about the correct way to handle those are.  As long as
> errno.h is included in defs.h, I see no reason to include it in any
> *.c file.  One could argue that errno.h should be remove from defs.h,
> but since errno (or the E* macros) is used in almost every file, I
> suspect we'd have to #include it in many files.

I'm aware of two strategies:

	o	Every module include a
		single header file and
		that suck in all the other
		header files.

		Makes for very simple but
		absolutely lethal make file
		dependencies :-)

	o	Every module define their
		own header and each component
		pull in the header files it
		is using.

		A variation on this theme has
		each of these header files
		[not] sucking in anything they
		refer to.

GDB clearly has a foot in both of those camps (and the second foot has
both of those variations squished between its toes .... :-).

"defs.h" contains declarations that various modules can't live without. 
Each module header contains declarations specific to their module.  How
self contained a module header file is,  is pretty arbitrary.

To get back to your question.  I expect the status quo to largely
remain:

	o	defs.h provide declarations for
		the things that all of GDB uses.

	o	module headers try to be largely
		self contained.  If they refer
		to other headers then suck
		those headers in.

So <errno.h> should be included but (to give a counter example)
<endian.h> should be given the boot (see TODO file for details).

	Andrew


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFA]: Remove unused header files.
  2001-03-01 17:44 [RFA]: Remove unused header files J.T. Conklin
                   ` (3 preceding siblings ...)
  2001-03-05  8:11 ` Andrew Cagney
@ 2001-03-05  8:11 ` Andrew Cagney
  2001-03-06 12:41   ` J.T. Conklin
  2001-03-05  8:11 ` Andrew Cagney
  5 siblings, 1 reply; 18+ messages in thread
From: Andrew Cagney @ 2001-03-05  8:11 UTC (permalink / raw)
  To: jtc; +Cc: gdb-patches

J.T.

I'm sorry if it appears that I'm bombarding your patch with an
increadable number of questions/comments.  It isn't that I've problems
with what you're trying to achieve - anything but!  It is just that I'm
trying to ensure that all issues have been clearly, and publically,
discussed.  

Remember, that will likely be me reminding people that GDB's convention
for ABC is XYZ ... :-)

enjoy,
	Andrew


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFA]: Remove unused header files.
  2001-03-01 17:44 [RFA]: Remove unused header files J.T. Conklin
  2001-03-02  8:25 ` Andrew Cagney
@ 2001-03-05  8:11 ` Andrew Cagney
  2001-03-06 13:10   ` J.T. Conklin
  2001-03-05  8:11 ` Andrew Cagney
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 18+ messages in thread
From: Andrew Cagney @ 2001-03-05  8:11 UTC (permalink / raw)
  To: jtc; +Cc: gdb-patches

>         * arch-utils.c (#include <ctype.h>): Removed.

This one caught my eye.  Given I wrote/cloned arch-utils.c and given
that I try to only include header files when they are needed (on the
basis of -Werror -W...) I'm wondering what went wrong :-)

	enjoy,
		Andrew


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFA]: Remove unused header files.
  2001-03-01 17:44 [RFA]: Remove unused header files J.T. Conklin
                   ` (2 preceding siblings ...)
  2001-03-05  8:11 ` Andrew Cagney
@ 2001-03-05  8:11 ` Andrew Cagney
  2001-03-06 12:38   ` J.T. Conklin
  2001-03-05  8:11 ` Andrew Cagney
  2001-03-05  8:11 ` Andrew Cagney
  5 siblings, 1 reply; 18+ messages in thread
From: Andrew Cagney @ 2001-03-05  8:11 UTC (permalink / raw)
  To: jtc; +Cc: gdb-patches

"J.T. Conklin" wrote:

> files.  Unlike the last time, this list was generated from a lint
> program

Just a thought, are you able to point at source for this program?

It may encourage others to use that same system and hence, help you in
your cause.  It would also mean that others can reproduce your
experimental results :-)

I should also be encouraging people to use Free tools :-)

	enjoy,
		Andrew


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFA]: Remove unused header files.
  2001-03-05  8:11 ` Andrew Cagney
@ 2001-03-06  9:20   ` Fernando Nasser
  2001-03-06 11:32     ` J.T. Conklin
  2001-03-06 13:27   ` J.T. Conklin
  1 sibling, 1 reply; 18+ messages in thread
From: Fernando Nasser @ 2001-03-06  9:20 UTC (permalink / raw)
  To: Andrew Cagney; +Cc: jtc, gdb-patches

> > The decode_line_1() declaration (IMO) indicates a unclear separation
> > of interface and implementation.  (Again, IMO) a function declaration
> > should only be in one header.  Please share your thoughts, on this
> > specific case, and in general.  decode_line_1() is the first of many
> > functions that have multiple declarations.
> 

I guess it should not be in symtab.c (old place), only in linespec.h (new place).
It may have been an oversight of my part.

Do you see any compilation failures if you remove the declaration of decode_line_1() from symtab.h?

-- 
Fernando Nasser
Red Hat - Toronto                       E-Mail:  fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario   M4P 2C9


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFA]: Remove unused header files.
  2001-03-06  9:20   ` Fernando Nasser
@ 2001-03-06 11:32     ` J.T. Conklin
  0 siblings, 0 replies; 18+ messages in thread
From: J.T. Conklin @ 2001-03-06 11:32 UTC (permalink / raw)
  To: Fernando Nasser; +Cc: gdb-patches

>>>>> "Fernando" == Fernando Nasser <fnasser@cygnus.com> writes:
>> > The decode_line_1() declaration (IMO) indicates a unclear separation
>> > of interface and implementation.  (Again, IMO) a function declaration
>> > should only be in one header.  Please share your thoughts, on this
>> > specific case, and in general.  decode_line_1() is the first of many
>> > functions that have multiple declarations.


Fernando> I guess it should not be in symtab.c (old place), only in
Fernando> linespec.h (new place).  It may have been an oversight of my
Fernando> part.

No biggie.  

Fernando> Do you see any compilation failures if you remove the
Fernando> declaration of decode_line_1() from symtab.h?

Now that I know where the declaration is supposed to live, I'll try it
out.

        --jtc

-- 
J.T. Conklin
RedBack Networks


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFA]: Remove unused header files.
  2001-03-05  8:11 ` Andrew Cagney
@ 2001-03-06 12:38   ` J.T. Conklin
  0 siblings, 0 replies; 18+ messages in thread
From: J.T. Conklin @ 2001-03-06 12:38 UTC (permalink / raw)
  To: Andrew Cagney; +Cc: gdb-patches

>>>>> "Andrew" == Andrew Cagney <ac131313@cygnus.com> writes:
>> files.  Unlike the last time, this list was generated from a lint
>> program

Andrew> Just a thought, are you able to point at source for this program?

Unfortunately not.  It's a commercial package, which is why I've
haven't mentioned it by name on the list.

There is a similar tool, LCLint, which is distributed under the GPL.
When I tried LCLint some years ago, I found it somewhat difficult to
get useful results.  This was especially true at the beggining of a
code cleanup project I was involved in when things were quite messy.

Andrew> It may encourage others to use that same system and hence, help you in
Andrew> your cause.  It would also mean that others can reproduce your
Andrew> experimental results :-)

Since being able to share results (and the effort too, if I'm lucky)
is important, I'm willing to give LClint another try.  It may be that
I'll have to take things a little further along with my package before
we can use it effectively.

Andrew> I should also be encouraging people to use Free tools :-)

If possible.  But if there are commercial packages that are available
without Free alternatives, I have no problems using them to develop
free software.

        --jtc

-- 
J.T. Conklin
RedBack Networks


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFA]: Remove unused header files.
  2001-03-05  8:11 ` Andrew Cagney
@ 2001-03-06 12:41   ` J.T. Conklin
  0 siblings, 0 replies; 18+ messages in thread
From: J.T. Conklin @ 2001-03-06 12:41 UTC (permalink / raw)
  To: Andrew Cagney; +Cc: gdb-patches

>>>>> "Andrew" == Andrew Cagney <ac131313@cygnus.com> writes:
Andrew> I'm sorry if it appears that I'm bombarding your patch with an
Andrew> increadable number of questions/comments.  It isn't that I've
Andrew> problems with what you're trying to achieve - anything but!
Andrew> It is just that I'm trying to ensure that all issues have been
Andrew> clearly, and publically, discussed.

I completely understand.  If we can come up with a include convention,
it makes decisions down the road a lot easier and less arbitrary.  I'm
not in a rush, because I believe rushed decisions often turn out to
have less than desirable consequences.

        --jtc

-- 
J.T. Conklin
RedBack Networks


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFA]: Remove unused header files.
  2001-03-05  8:11 ` Andrew Cagney
@ 2001-03-06 13:02   ` J.T. Conklin
  0 siblings, 0 replies; 18+ messages in thread
From: J.T. Conklin @ 2001-03-06 13:02 UTC (permalink / raw)
  To: Andrew Cagney; +Cc: gdb-patches

>>>>> "Andrew" == Andrew Cagney <ac131313@cygnus.com> writes:
>> I'm unsure about the correct way to handle those are.  As long as
>> errno.h is included in defs.h, I see no reason to include it in any
>> *.c file.  One could argue that errno.h should be remove from defs.h,
>> but since errno (or the E* macros) is used in almost every file, I
>> suspect we'd have to #include it in many files.

Andrew> I'm aware of two strategies:
Andrew>
Andrew> 	o	Every module include a
Andrew> 		single header file and
Andrew> 		that suck in all the other
Andrew> 		header files.

I've seen this technique used for small systems and it works well ---
that is until it grows into a medium sized or large system.  By then,
the effort to convert to a sane include convention is so large it may
never get done.  IMO GDB is far beyond the size where this could work.

Andrew> 	o	Every module define their
Andrew> 		own header and each component
Andrew> 		pull in the header files it
Andrew> 		is using.
Andrew>
Andrew> 		A variation on this theme has
Andrew> 		each of these header files
Andrew> 		[not] sucking in anything they
Andrew> 		refer to.

I'm in favor of this approach, including the variation where header
files include the headers containing any macros, declarations, etc.,
they refer to.  I've seen cases when this is not true, where a header
must be included before another because of some implementation detail
rather than a public interface.  When the implementation changes, the
header is no longer needed; but then all the source files need to be
updated.  This never seems to happen.

Andrew> To get back to your question.  I expect the status quo to
Andrew> largely remain:
Andrew>
Andrew> 	o	defs.h provide declarations for
Andrew> 		the things that all of GDB uses.

I agree.  I do think there could be some culling of declarations in
defs.h to module specific headers.

Andrew> 	o	module headers try to be largely
Andrew> 		self contained.  If they refer to other
Andrew> 		headers then suck those headers in.

Again I agree.  

In the absense of any objections, this sounds like this is a beginning
of a include convention.

Andrew> So <errno.h> should be included but (to give a counter example)
Andrew> <endian.h> should be given the boot (see TODO file for details).

So if we agree that <errno.h> should be included in defs.h, there is no
reason why it should not be removed from all the *.c files that include
defs.h.

        --jtc 

-- 
J.T. Conklin
RedBack Networks


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFA]: Remove unused header files.
  2001-03-05  8:11     ` Andrew Cagney
@ 2001-03-06 13:08       ` J.T. Conklin
  0 siblings, 0 replies; 18+ messages in thread
From: J.T. Conklin @ 2001-03-06 13:08 UTC (permalink / raw)
  To: Andrew Cagney; +Cc: gdb-patches

>>>>> "Andrew" == Andrew Cagney <ac131313@cygnus.com> writes:
>> Yes and no.  If I was linting GDB configured for the mn10200, it
>> regcache.h would be considered necessary, but for other cases it
>> would not.  Since I'm not planning on linting every possible config
>> before deciding whether a header is unnecessary, it's not up to the
>> job.

Andrew> I think we need to clarify one of the ground rules - should
Andrew> "tm.h" et.al. files be sucking in the things they refer to and
Andrew> (assuming that is agreed) should that be addressed before
Andrew> include headers are eliminated from .c files.

Andrew> If that isn't done, I've the (rational) fear that these
Andrew> cleanups will start breaking an unreasonable number of
Andrew> existing targets.  Rational?  Well I'm still cleaning up
Andrew> fallout from regcache.h - I've ~50 tm.h files that should
Andrew> include regcache.h.

I think you know my position already.  I think it is reasonable to
require that tm-, nm-, and xm- headers to include any headers that
contain definitions, declarations, etc. that are needed.  

As a result, this lifts the requirement that all modules must include
all header files that any (of the zillion) tm-, nm-, or xm- header
depends on.

        --jtc

-- 
J.T. Conklin
RedBack Networks


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFA]: Remove unused header files.
  2001-03-05  8:11 ` Andrew Cagney
@ 2001-03-06 13:10   ` J.T. Conklin
  0 siblings, 0 replies; 18+ messages in thread
From: J.T. Conklin @ 2001-03-06 13:10 UTC (permalink / raw)
  To: Andrew Cagney; +Cc: gdb-patches

>>>>> "Andrew" == Andrew Cagney <ac131313@cygnus.com> writes:
>> * arch-utils.c (#include <ctype.h>): Removed.

Andrew> This one caught my eye.  Given I wrote/cloned arch-utils.c and given
Andrew> that I try to only include header files when they are needed (on the
Andrew> basis of -Werror -W...) I'm wondering what went wrong :-)

No idea.  Perhaps it initially needed ctype.h and has since been
changed?

        --jtc

-- 
J.T. Conklin
RedBack Networks


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFA]: Remove unused header files.
  2001-03-05  8:11 ` Andrew Cagney
  2001-03-06  9:20   ` Fernando Nasser
@ 2001-03-06 13:27   ` J.T. Conklin
  2001-03-06 15:06     ` Andrew Cagney
  1 sibling, 1 reply; 18+ messages in thread
From: J.T. Conklin @ 2001-03-06 13:27 UTC (permalink / raw)
  To: Andrew Cagney; +Cc: gdb-patches

>>>>> "Andrew" == Andrew Cagney <ac131313@cygnus.com> writes:
>> The decode_line_1() declaration (IMO) indicates a unclear separation
>> of interface and implementation.  (Again, IMO) a function declaration
>> should only be in one header.  Please share your thoughts, on this
>> specific case, and in general.  decode_line_1() is the first of many
>> functions that have multiple declarations.

Andrew> Only one? Just don't get me started on the number of extern
Andrew> declarations that appear in .c files! :-) Yes, I agree that
Andrew> function declarations should only appear once.

Is this one of the things your static analysis database is checks for
(And can you send me a pointer to it again)?  

When I was going through the code, I saw one comment describing how a
few functions were being declared locally so the header containing the
declarations would not have to be included.  This seems like a recipe
for disaster.  If the interface is ever changed, the programmer might
miss the local declaration when fixing up the callers.  And the compiler
won't complain because the usage within that function will match the
(local) declarations.  Bleh.

If it was my rule, function declarations should only appear once and 
should *never* occur in *.c files.

Andrew> As for the more general question of confused interfaces.  I
Andrew> think GDB's internal structure is facing major change
Andrew> (multi-arch).  Consequently I'm hopeing that that that that
Andrew> work will address many of those problems.  I suspect that, at
Andrew> this stage, you might want to put blinkers and just
Andrew> concentrate on the basic #include mess.  That, by its self, is
Andrew> pretty daunting.

Note I'm not trying to fix everything I come across, at least not in
the first pass.  For the most part, I am trying to categorize things 
so I know what's the most benefit for the least effort.

The side-effect of this is that I'll likely be raising a lot of issues
wrt inconsistancies.  I won't be offended if you tell me to punt on
any of them.

        --jtc

-- 
J.T. Conklin
RedBack Networks


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFA]: Remove unused header files.
  2001-03-06 13:27   ` J.T. Conklin
@ 2001-03-06 15:06     ` Andrew Cagney
  0 siblings, 0 replies; 18+ messages in thread
From: Andrew Cagney @ 2001-03-06 15:06 UTC (permalink / raw)
  To: jtc; +Cc: gdb-patches

"J.T. Conklin" wrote:
> 
> >>>>> "Andrew" == Andrew Cagney <ac131313@cygnus.com> writes:
> >> The decode_line_1() declaration (IMO) indicates a unclear separation
> >> of interface and implementation.  (Again, IMO) a function declaration
> >> should only be in one header.  Please share your thoughts, on this
> >> specific case, and in general.  decode_line_1() is the first of many
> >> functions that have multiple declarations.
> 
> Andrew> Only one? Just don't get me started on the number of extern
> Andrew> declarations that appear in .c files! :-) Yes, I agree that
> Andrew> function declarations should only appear once.
> 
> Is this one of the things your static analysis database is checks for
> (And can you send me a pointer to it again)?

Static analysis database? You mean that big nasty (probably broken) awk
script?

http://sources.redhat.com/gdb/ari/

It checks it but doesn't list it (well not by default).

	Andrew


^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2001-03-06 15:06 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2001-03-01 17:44 [RFA]: Remove unused header files J.T. Conklin
2001-03-02  8:25 ` Andrew Cagney
2001-03-02 16:34   ` J.T. Conklin
2001-03-05  8:11     ` Andrew Cagney
2001-03-06 13:08       ` J.T. Conklin
2001-03-05  8:11 ` Andrew Cagney
2001-03-06 13:10   ` J.T. Conklin
2001-03-05  8:11 ` Andrew Cagney
2001-03-06 13:02   ` J.T. Conklin
2001-03-05  8:11 ` Andrew Cagney
2001-03-06 12:38   ` J.T. Conklin
2001-03-05  8:11 ` Andrew Cagney
2001-03-06 12:41   ` J.T. Conklin
2001-03-05  8:11 ` Andrew Cagney
2001-03-06  9:20   ` Fernando Nasser
2001-03-06 11:32     ` J.T. Conklin
2001-03-06 13:27   ` J.T. Conklin
2001-03-06 15:06     ` Andrew Cagney

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox