FYI: minsyms documentation

FYI: minsyms documentation

[Tom Tromey]

I am checking this in on the trunk.

Today I decided to try to document the minsyms API more or less the way
I would like APIs to be documented in general.  This patch implements
that; it move documentation from function definitions to minsyms.h, adds
an introductory comment about minsyms there as well, and it rearranges
the header into a more logical order.

My view is that gdbint.texinfo should eventually hold a high-level
overview of the different modules in GDB, but that each individual
module should be documented in the relevant header files.  My reason for
this is just that it is simpler to update documentation when it is in
the form of comments.  I think gdbint.texinfo should also hold
documentation on our procedures, coding styles, and other things that
are not directly related to some piece of code.

In the past people have mentioned using something like Doxygen to
extract an actual document from the sources.  I am not opposed to this,
but I think that, as GDB is a program and not a library, such
documentation is of limited value; and in any case there are known
licensing issues with any such scheme -- see the archives of the GCC
lists for details.

Tom

2011-12-21  Tom Tromey  <tromey@redhat.com>

	* minsyms.h: Rearrange.  Document header and all functions.
	* minsyms.c: Move some comments to minsyms.h.

Index: minsyms.c
===================================================================
RCS file: /cvs/src/src/gdb/minsyms.c,v
retrieving revision 1.87
diff -u -r1.87 minsyms.c
--- minsyms.c	21 Dec 2011 21:51:56 -0000	1.87
+++ minsyms.c	21 Dec 2011 22:03:34 -0000
@@ -78,7 +78,7 @@
 
 static int msym_count;
 
-/* Compute a hash code based using the same criteria as `strcmp_iw'.  */
+/* See minsyms.h.  */
 
 unsigned int
 msymbol_hash_iw (const char *string)
@@ -98,7 +98,7 @@
   return hash;
 }
 
-/* Compute a hash code for a string.  */
+/* See minsyms.h.  */
 
 unsigned int
 msymbol_hash (const char *string)
@@ -141,8 +141,8 @@
     }
 }
 
+/* See minsyms.h.  */
 
-/* Return OBJFILE where minimal symbol SYM is defined.  */
 struct objfile *
 msymbol_objfile (struct minimal_symbol *sym)
 {
@@ -310,13 +310,7 @@
   return NULL;
 }
 
-/* Iterate over all the minimal symbols in the objfile OBJF which
-   match NAME.  Both the ordinary and demangled names of each symbol
-   are considered.  The caller is responsible for canonicalizing NAME,
-   should that need to be done.
-   
-   For each matching symbol, CALLBACK is called with the symbol and
-   USER_DATA as arguments.  */
+/* See minsyms.h.  */
 
 void
 iterate_over_minimal_symbols (struct objfile *objf, const char *name,
@@ -350,12 +344,7 @@
     }
 }
 
-/* Look through all the current minimal symbol tables and find the
-   first minimal symbol that matches NAME and has text type.  If OBJF
-   is non-NULL, limit the search to that objfile.  Returns a pointer
-   to the minimal symbol that matches, or NULL if no match is found.
-
-   This function only searches the mangled (linkage) names.  */
+/* See minsyms.h.  */
 
 struct minimal_symbol *
 lookup_minimal_symbol_text (const char *name, struct objfile *objf)
@@ -407,10 +396,7 @@
   return NULL;
 }
 
-/* Look through all the current minimal symbol tables and find the
-   first minimal symbol that matches NAME and PC.  If OBJF is non-NULL,
-   limit the search to that objfile.  Returns a pointer to the minimal
-   symbol that matches, or NULL if no match is found.  */
+/* See minsyms.h.  */
 
 struct minimal_symbol *
 lookup_minimal_symbol_by_pc_name (CORE_ADDR pc, const char *name,
@@ -442,13 +428,7 @@
   return NULL;
 }
 
-/* Look through all the current minimal symbol tables and find the
-   first minimal symbol that matches NAME and is a solib trampoline.
-   If OBJF is non-NULL, limit the search to that objfile.  Returns a
-   pointer to the minimal symbol that matches, or NULL if no match is
-   found.
-
-   This function only searches the mangled (linkage) names.  */
+/* See minsyms.h.  */
 
 struct minimal_symbol *
 lookup_minimal_symbol_solib_trampoline (const char *name,
@@ -734,8 +714,7 @@
   return lookup_minimal_symbol_by_pc_section_1 (pc, section, 0);
 }
 
-/* Backward compatibility: search through the minimal symbol table 
-   for a matching PC (no section given).  */
+/* See minsyms.h.  */
 
 struct minimal_symbol *
 lookup_minimal_symbol_by_pc (CORE_ADDR pc)
@@ -806,10 +785,7 @@
 
 const struct gnu_ifunc_fns *gnu_ifunc_fns_p = &stub_gnu_ifunc_fns;
 
-/* Find the minimal symbol named NAME, and return both the minsym
-   struct and its objfile.  This only checks the linkage name.  Sets
-   *OBJFILE_P and returns the minimal symbol, if it is found.  If it
-   is not found, returns NULL.  */
+/* See minsyms.h.  */
 
 struct minimal_symbol *
 lookup_minimal_symbol_and_objfile (const char *name,
@@ -853,18 +829,21 @@
   return 0;
 }
 
-/* Prepare to start collecting minimal symbols.  Note that presetting
-   msym_bunch_index to BUNCH_SIZE causes the first call to save a minimal
-   symbol to allocate the memory for the first bunch.  */
+/* See minsyms.h.  */
 
 void
 init_minimal_symbol_collection (void)
 {
   msym_count = 0;
   msym_bunch = NULL;
+  /* Note that presetting msym_bunch_index to BUNCH_SIZE causes the
+     first call to save a minimal symbol to allocate the memory for
+     the first bunch.  */
   msym_bunch_index = BUNCH_SIZE;
 }
 
+/* See minsyms.h.  */
+
 void
 prim_record_minimal_symbol (const char *name, CORE_ADDR address,
 			    enum minimal_symbol_type ms_type,
@@ -896,8 +875,7 @@
 				       section, NULL, objfile);
 }
 
-/* Record a minimal symbol in the msym bunches.  Returns the symbol
-   newly created.  */
+/* See minsyms.h.  */
 
 struct minimal_symbol *
 prim_record_minimal_symbol_full (const char *name, int name_len, int copy_name,
@@ -974,8 +952,7 @@
   return msymbol;
 }
 
-/* Record a minimal symbol in the msym bunches.  Returns the symbol
-   newly created.  */
+/* See minsyms.h.  */
 
 struct minimal_symbol *
 prim_record_minimal_symbol_and_info (const char *name, CORE_ADDR address,
@@ -1048,6 +1025,8 @@
     }
 }
 
+/* See minsyms.h.  */
+
 struct cleanup *
 make_cleanup_discard_minimal_symbols (void)
 {
@@ -1322,9 +1301,7 @@
   build_minimal_symbol_hash_tables (objfile);
 }
 
-/* Check if PC is in a shared library trampoline code stub.
-   Return minimal symbol for the trampoline entry or NULL if PC is not
-   in a trampoline code stub.  */
+/* See minsyms.h.  */
 
 struct minimal_symbol *
 lookup_solib_trampoline_symbol_by_pc (CORE_ADDR pc)
Index: minsyms.h
===================================================================
RCS file: /cvs/src/src/gdb/minsyms.h,v
retrieving revision 1.1
diff -u -r1.1 minsyms.h
--- minsyms.h	21 Dec 2011 21:51:56 -0000	1.1
+++ minsyms.h	21 Dec 2011 22:03:34 -0000
@@ -20,20 +20,86 @@
 #ifndef MINSYMS_H
 #define MINSYMS_H
 
-/* Functions for dealing with the minimal symbol table, really a misc
-   address<->symbol mapping for things we don't have debug symbols for.  */
+/* This header declares most of the API for dealing with minimal
+   symbols and minimal symbol tables.  A few things are declared
+   elsewhere; see below.
+
+   A minimal symbol is a symbol for which there is no direct debug
+   information.  For example, for an ELF binary, minimal symbols are
+   created from the ELF symbol table.
+
+   For the definition of the minimal symbol structure, see struct
+   minimal_symbol in symtab.h.
+
+   Minimal symbols are stored in tables attached to an objfile; see
+   objfiles.h for details.  Code should generally treat these tables
+   as opaque and use functions provided by minsyms.c to inspect them.
+*/
+
+/* Prepare to start collecting minimal symbols.  This should be called
+   by a symbol reader to initialize the minimal symbol module.
+   Currently, minimal symbol table creation is not reentrant; it
+   relies on global (static) variables in minsyms.c.  */
 
-void prim_record_minimal_symbol (const char *, CORE_ADDR,
-				 enum minimal_symbol_type,
-				 struct objfile *);
+void init_minimal_symbol_collection (void);
+
+/* Return a cleanup which is used to clean up the global state left
+   over by minimal symbol creation.  After calling
+   init_minimal_symbol_collection, a symbol reader should call this
+   function.  Then, after all minimal symbols have been read,
+   regardless of whether they are installed or not, the cleanup
+   returned by this function should be run.  */
+
+struct cleanup *make_cleanup_discard_minimal_symbols (void);
+
+/* Record a new minimal symbol.  This is the "full" entry point;
+   simpler convenience entry points are also provided below.
+   
+   This returns a new minimal symbol.  It is ok to modify the returned
+   minimal symbol (though generally not necessary).  It is not ok,
+   though, to stash the pointer anywhere; as minimal symbols may be
+   moved after creation.  The memory for the returned minimal symbol
+   is still owned by the minsyms.c code, and should not be freed.
+   
+   Arguments are:
+
+   NAME - the symbol's name
+   NAME_LEN - the length of the name
+   COPY_NAME - if true, the minsym code must make a copy of NAME.  If
+   false, then NAME must be NUL-terminated, and must have a lifetime
+   that is at least as long as OBJFILE's lifetime.
+   ADDRESS - the address of the symbol
+   MS_TYPE - the type of the symbol
+   SECTION - the symbol's section
+   BFD_SECTION - the symbol's BFD section; used to find the
+   appropriate obj_section for the minimal symbol.  This can be NULL.
+   OBJFILE - the objfile associated with the minimal symbol.  */
 
 struct minimal_symbol *prim_record_minimal_symbol_full
-    (const char *,
-     int, int, CORE_ADDR,
-     enum minimal_symbol_type,
+    (const char *name,
+     int name_len,
+     int copy_name,
+     CORE_ADDR address,
+     enum minimal_symbol_type ms_type,
      int section,
      asection *bfd_section,
-     struct objfile *);
+     struct objfile *objfile);
+
+/* Like prim_record_minimal_symbol_full, but:
+   - uses strlen to compute NAME_LEN,
+   - passes COPY_NAME = 0,
+   - passes SECTION = 0,
+   - and passes BFD_SECTION = NULL.
+   
+   This variant does not return the new symbol.  */
+
+void prim_record_minimal_symbol (const char *, CORE_ADDR,
+				 enum minimal_symbol_type,
+				 struct objfile *);
+
+/* Like prim_record_minimal_symbol_full, but:
+   - uses strlen to compute NAME_LEN,
+   - passes COPY_NAME = 0.  */
 
 struct minimal_symbol *prim_record_minimal_symbol_and_info
     (const char *,
@@ -43,10 +109,37 @@
      asection *bfd_section,
      struct objfile *);
 
-unsigned int msymbol_hash_iw (const char *);
+/* Install the minimal symbols that have been collected into the given
+   objfile.  After this is called, the cleanup returned by
+   make_cleanup_discard_minimal_symbols should be run in order to
+   clean up global state.  */
+
+void install_minimal_symbols (struct objfile *);
+
+/* Create the terminating entry of OBJFILE's minimal symbol table.
+   If OBJFILE->msymbols is zero, allocate a single entry from
+   OBJFILE->objfile_obstack; otherwise, just initialize
+   OBJFILE->msymbols[OBJFILE->minimal_symbol_count].  */
+
+void terminate_minimal_symbol_table (struct objfile *objfile);
+
+/* Sort all the minimal symbols in OBJFILE.  This should be only be
+   called after relocating symbols; it ensures that the minimal
+   symbols are properly sorted by address.  */
+
+void msymbols_sort (struct objfile *objfile);
+
+\f
+
+/* Compute a hash code for the string argument.  */
 
 unsigned int msymbol_hash (const char *);
 
+/* Like msymbol_hash, but compute a hash code that is compatible with
+   strcmp_iw.  */
+
+unsigned int msymbol_hash_iw (const char *);
+
 /* Compute the next hash value from previous HASH and the character C.  This
    is only a GDB in-memory computed value with no external files compatibility
    requirements.  */
@@ -54,49 +147,100 @@
 #define SYMBOL_HASH_NEXT(hash, c)			\
   ((hash) * 67 + tolower ((unsigned char) (c)) - 113)
 
+\f
+
+/* Return the objfile that holds the minimal symbol SYM.  Every
+   minimal symbols is held by some objfile; this will never return
+   NULL.  */
+
 struct objfile *msymbol_objfile (struct minimal_symbol *sym);
 
+\f
+
+/* Look through all the current minimal symbol tables and find the
+   first minimal symbol that matches NAME.  If OBJF is non-NULL, limit
+   the search to that objfile.  If SFILE is non-NULL, the only file-scope
+   symbols considered will be from that source file (global symbols are
+   still preferred).  Returns a pointer to the minimal symbol that
+   matches, or NULL if no match is found.  */
+
 struct minimal_symbol *lookup_minimal_symbol (const char *,
 					      const char *,
 					      struct objfile *);
 
+/* Find the minimal symbol named NAME, and return both the minsym
+   struct and its objfile.  This only checks the linkage name.  Sets
+   *OBJFILE_P and returns the minimal symbol, if it is found.  If it
+   is not found, returns NULL.  */
+
+struct minimal_symbol *lookup_minimal_symbol_and_objfile (const char *,
+							  struct objfile **);
+
+/* Look through all the current minimal symbol tables and find the
+   first minimal symbol that matches NAME and has text type.  If OBJF
+   is non-NULL, limit the search to that objfile.  Returns a pointer
+   to the minimal symbol that matches, or NULL if no match is found.
+
+   This function only searches the mangled (linkage) names.  */
+
 struct minimal_symbol *lookup_minimal_symbol_text (const char *,
 						   struct objfile *);
 
+/* Look through all the current minimal symbol tables and find the
+   first minimal symbol that matches NAME and is a solib trampoline.
+   If OBJF is non-NULL, limit the search to that objfile.  Returns a
+   pointer to the minimal symbol that matches, or NULL if no match is
+   found.
+
+   This function only searches the mangled (linkage) names.  */
+
 struct minimal_symbol *lookup_minimal_symbol_solib_trampoline
     (const char *,
      struct objfile *);
 
+/* Look through all the current minimal symbol tables and find the
+   first minimal symbol that matches NAME and PC.  If OBJF is non-NULL,
+   limit the search to that objfile.  Returns a pointer to the minimal
+   symbol that matches, or NULL if no match is found.  */
+
 struct minimal_symbol *lookup_minimal_symbol_by_pc_name
-(CORE_ADDR, const char *, struct objfile *);
+    (CORE_ADDR, const char *, struct objfile *);
 
-struct minimal_symbol *lookup_minimal_symbol_by_pc (CORE_ADDR);
+/* Search through the minimal symbol table for each objfile and find
+   the symbol whose address is the largest address that is still less
+   than or equal to PC, and which matches SECTION.
 
-struct minimal_symbol *lookup_minimal_symbol_and_objfile (const char *,
-							  struct objfile **);
+   If SECTION is NULL, this uses the result of find_pc_section
+   instead.
+
+   Returns a pointer to the minimal symbol if such a symbol is found,
+   or NULL if PC is not in a suitable range.  */
 
 struct minimal_symbol *lookup_minimal_symbol_by_pc_section
     (CORE_ADDR,
      struct obj_section *);
 
-struct minimal_symbol *lookup_solib_trampoline_symbol_by_pc (CORE_ADDR);
-
-void init_minimal_symbol_collection (void);
-
-struct cleanup *make_cleanup_discard_minimal_symbols (void);
+/* Backward compatibility: search through the minimal symbol table 
+   for a matching PC (no section given).
+   
+   This is a wrapper that calls lookup_minimal_symbol_by_pc_section
+   with a NULL section argument.  */
 
-void install_minimal_symbols (struct objfile *);
+struct minimal_symbol *lookup_minimal_symbol_by_pc (CORE_ADDR);
 
-/* Sort all the minimal symbols in OBJFILE.  */
+/* Check if PC is in a shared library trampoline code stub.
+   Return minimal symbol for the trampoline entry or NULL if PC is not
+   in a trampoline code stub.  */
 
-void msymbols_sort (struct objfile *objfile);
+struct minimal_symbol *lookup_solib_trampoline_symbol_by_pc (CORE_ADDR);
 
-/* Create the terminating entry of OBJFILE's minimal symbol table.
-   If OBJFILE->msymbols is zero, allocate a single entry from
-   OBJFILE->objfile_obstack; otherwise, just initialize
-   OBJFILE->msymbols[OBJFILE->minimal_symbol_count].  */
+/* Iterate over all the minimal symbols in the objfile OBJF which
+   match NAME.  Both the ordinary and demangled names of each symbol
+   are considered.  The caller is responsible for canonicalizing NAME,
+   should that need to be done.
 
-void terminate_minimal_symbol_table (struct objfile *objfile);
+   For each matching symbol, CALLBACK is called with the symbol and
+   USER_DATA as arguments.  */
 
 void iterate_over_minimal_symbols (struct objfile *objf,
 				   const char *name,

Re: FYI: minsyms documentation

[Joel Brobecker]

> Today I decided to try to document the minsyms API more or less the way
> I would like APIs to be documented in general.  This patch implements
> that; it move documentation from function definitions to minsyms.h, adds
> an introductory comment about minsyms there as well, and it rearranges
> the header into a more logical order.

OK. Let's move in that direction. I think it makes sense, the API should
tell the developer how it expects to be used. Keeping the "See header"
comment is important too, IMO, makes it easier to verify that a given
function is in fact documeted or not.

-- 
Joel

Re: FYI: minsyms documentation

[Stan Shebs]

On 12/21/11 6:34 PM, Tom Tromey wrote:
> I am checking this in on the trunk.
>
> Today I decided to try to document the minsyms API more or less the way
> I would like APIs to be documented in general.  This patch implements
> that; it move documentation from function definitions to minsyms.h, adds
> an introductory comment about minsyms there as well, and it rearranges
> the header into a more logical order.

I'm not liking this idea very much I'm afraid.

First, as you point out later, GDB is not really a library.  So internal 
APIs tend to be pretty fluid, depending on the needs of the moment.  But 
if you mandate that the documentation be in different places depending 
on function visibility, then the addition or removal of a static keyword 
must be accompanied by a move of the header comment block.  Even if one 
did nothing more than split an overly-large file (and I note we now have 
five source files > 10Kloc, long overdue for breakup), then there would 
have to be massive movement of comment blocks, even if none of the 
functions actually changed.  Plus we already have many functions 
declared in headers that really are intended to be private, and are only 
public because of C's limitations; we don't really want to document 
those as if they are stable API.

Second, this is potentially a very large change to the sources, but if 
it's incremental, then we get into a confusing situation where some 
files are changed, others are not, and some headers are half-changed 
because they service multiple source files.  I've been digging through 
the bowels of MI code lately, and am reminded of how many of Andrew's 
initiatives were never completed, generating all kinds of uncertainty 
about whether to try to finish them, or roll back, or what.  For things 
that are major stylistic changes, we should at least think about whether 
we can do some kind of automated transformation that brings all the code 
into a new consistent state.

> My view is that gdbint.texinfo should eventually hold a high-level
> overview of the different modules in GDB, but that each individual
> module should be documented in the relevant header files.  My reason for
> this is just that it is simpler to update documentation when it is in
> the form of comments.  I think gdbint.texinfo should also hold
> documentation on our procedures, coding styles, and other things that
> are not directly related to some piece of code.

Isn't that generally our working assumption now?

> In the past people have mentioned using something like Doxygen to
> extract an actual document from the sources.  I am not opposed to this,
> but I think that, as GDB is a program and not a library, such
> documentation is of limited value; and in any case there are known
> licensing issues with any such scheme -- see the archives of the GCC
> lists for details.

I agree, Doxygen is not very plausible.  It could even be misleading, as 
Doxygen output tends to make functions look more self-contained and 
well-defined than would actually be the case for most of GDB.

Stan

Re: FYI: minsyms documentation

[Tom Tromey]

>>>>> "Stan" == Stan Shebs <stanshebs@earthlink.net> writes:

Stan> On 12/21/11 6:34 PM, Tom Tromey wrote:
>> I am checking this in on the trunk.
>> 
>> Today I decided to try to document the minsyms API more or less the way
>> I would like APIs to be documented in general.  This patch implements
>> that; it move documentation from function definitions to minsyms.h, adds
>> an introductory comment about minsyms there as well, and it rearranges
>> the header into a more logical order.

Stan> I'm not liking this idea very much I'm afraid.

Ok.

Do you mean you want me to back out the patch?
Let me know.

Stan> Second, this is potentially a very large change to the sources, but if
Stan> it's incremental, then we get into a confusing situation where some
Stan> files are changed, others are not, and some headers are half-changed
Stan> because they service multiple source files.

This is the present situation.

Tom> My view is that gdbint.texinfo should eventually hold a high-level
Tom> overview of the different modules in GDB, but that each individual
Tom> module should be documented in the relevant header files.  My reason for
Tom> this is just that it is simpler to update documentation when it is in
Tom> the form of comments.  I think gdbint.texinfo should also hold
Tom> documentation on our procedures, coding styles, and other things that
Tom> are not directly related to some piece of code.

Stan> Isn't that generally our working assumption now?

My working assumption is that gdbint.texinfo is barely maintained at
all.

Tom

Re: FYI: minsyms documentation

[Eli Zaretskii]

> From: Tom Tromey <tromey@redhat.com>
> Cc: gdb-patches@sourceware.org
> Date: Thu, 22 Dec 2011 13:13:19 -0700
> 
> My working assumption is that gdbint.texinfo is barely maintained at
> all.

Only because none of the active developers want to document GDB
internals in a Texinfo manual.  Therefore, the sad state of
gdbint.texinfo is a testament of what the majority of GDB maintainers
think it should look like.

Re: FYI: minsyms documentation

[Stan Shebs]

On 12/22/11 12:13 PM, Tom Tromey wrote:
>>>>>> "Stan" == Stan Shebs<stanshebs@earthlink.net>  writes:
> Stan>  On 12/21/11 6:34 PM, Tom Tromey wrote:
>>> I am checking this in on the trunk.
>>>
>>> Today I decided to try to document the minsyms API more or less the way
>>> I would like APIs to be documented in general.  This patch implements
>>> that; it move documentation from function definitions to minsyms.h, adds
>>> an introductory comment about minsyms there as well, and it rearranges
>>> the header into a more logical order.
> Stan>  I'm not liking this idea very much I'm afraid.
>
> Ok.
>
> Do you mean you want me to back out the patch?
> Let me know.

We don't need to be that hasty. :-)  I'd like to see more thoughts and 
reactions; if everybody else likes it, I won't stand in the way of progress!

>
> Stan>  Second, this is potentially a very large change to the sources, but if
> Stan>  it's incremental, then we get into a confusing situation where some
> Stan>  files are changed, others are not, and some headers are half-changed
> Stan>  because they service multiple source files.
>
> This is the present situation.

True, and certainly we should provide some sort of guidance on the point.

One way to look at it is if we assume that the transition to C++ occurs 
at some point, what happens to current function head comments?  Do they 
tend to stay put, as explanation of a method's implementation, or do 
they tend to migrate to class definitions in headers?  Perhaps method 
implementations won't often need head comment blocks at all, as the 
interface is documented in headers, and anything interesting in the 
implementation is described with inline comments as is is now.

Stan

Re: FYI: minsyms documentation

[Stan Shebs]

On 12/22/11 12:49 PM, Eli Zaretskii wrote:
>> From: Tom Tromey<tromey@redhat.com>
>> Cc: gdb-patches@sourceware.org
>> Date: Thu, 22 Dec 2011 13:13:19 -0700
>>
>> My working assumption is that gdbint.texinfo is barely maintained at
>> all.
> Only because none of the active developers want to document GDB
> internals in a Texinfo manual.  Therefore, the sad state of
> gdbint.texinfo is a testament of what the majority of GDB maintainers
> think it should look like.
>

Perhaps that means we should rethink whether we need gdbint.texinfo at 
all.  If everybody is able to madly hack away at the code without ever 
consulting the internals manual, then what purpose is it serving 
exactly?  Are newbies learning by reading the manual, or reading the 
code?  If the latter, then gdbint.texinfo content might get more 
attention if it was redistributed into 1-2 page blocks at the tops of 
relevant source files.

Stan

Re: FYI: minsyms documentation

[Joel Brobecker]

> I'm not liking this idea very much I'm afraid.

I don't have a strong opinion on this. I think you made some
good comments on the downside of this approach. But I also
think it does have some advantages as well.

I do think that...

> Doxygen output tends to make functions look more self-contained and
> well-defined than would actually be the case for most of GDB.

... is a weakness of our design, which makes it harder to use
as justification for another decision.

For me, I came to enjoy the fact that function descriptions can
always be found by locating the function implementation, which
in most cases is unique, and can be found using grep. But I also
recognize that many a times I have searched through the routines
applying to a given "object" searching for something that might
do what I want to do. Not having the documentation right next
to it was a real productivity hit.

So, perhaps the right approach lies in the middle. Only apply
Tom's approach to parts where it should in fact be an API. Things
like gdb_usleep, or vec.h are obvious candidates, for instance.
You've got to love how addrmap.h is documented, making it much
easier to use that module. And in the same vein, the "object"-like
structures that have use in GDB (symbols, symbol tables, values,
etc) could be documented similarly.

I should also mention this: One approach, followed by GNAT
engineers, is that they always have a declaration, either
in the package spec (header) or at the beginning of the package
body (.c file). And they always provide the function description
where the declaration is made. Should a function become public,
they move the whole blob, declaration and documentation to the
spec. I am not a fan, but if people like it, we can experiment
with that idea.

One thing I would like, though, is for all of us to insist on
every new function being documented. Even some GMs do not follow
or enforce this rule...

-- 
Joel

Re: FYI: minsyms documentation

[Eli Zaretskii]

> Date: Thu, 22 Dec 2011 13:17:57 -0800
> From: Stan Shebs <stanshebs@earthlink.net>
> 
> On 12/22/11 12:49 PM, Eli Zaretskii wrote:
> >> From: Tom Tromey<tromey@redhat.com>
> >> Cc: gdb-patches@sourceware.org
> >> Date: Thu, 22 Dec 2011 13:13:19 -0700
> >>
> >> My working assumption is that gdbint.texinfo is barely maintained at
> >> all.
> > Only because none of the active developers want to document GDB
> > internals in a Texinfo manual.  Therefore, the sad state of
> > gdbint.texinfo is a testament of what the majority of GDB maintainers
> > think it should look like.
> >
> 
> Perhaps that means we should rethink whether we need gdbint.texinfo at 
> all.

That came up as well, you can find the discussions in the archives.

> If everybody is able to madly hack away at the code without ever 
> consulting the internals manual, then what purpose is it serving 
> exactly?

In its current condition, I don't see whom it can serve, except as
misinformation.

> Are newbies learning by reading the manual, or reading the code?

What newbies?  The people who hack at the core features of GDB can be
counted on fingers of a single hand, and they didn't change in years.

> If the latter, then gdbint.texinfo content might get more attention
> if it was redistributed into 1-2 page blocks at the tops of relevant
> source files.

Again, the current content of that manual can only do a mis-service,
so redistributing it is wasted effort.  If we want to have this kind
of material _anywhere_, it must be maintained, i.e. at the very least
kept in sync with the development.

Documenting complex software in comments is very sub-optimal.  It can
document each separate API, but it cannot have too much context, and
it cannot present the overall design of each feature.  It also lacks
the means, like hyperlinks, to refer the reader to another place.
Linear text performs poorly both as a tutorial introduction and as
reference material.

You can see all these problems in the proposed minsyms.h, and even in
addrmap.h (where clearly the authors went out of their way in
providing context: 100+ lines for just 7 APIs!).  What we have there
is a detailed description of a series of related APIs, but no
information about _why_ these APIs are needed, _when_ they should be
used, and how they interact with other relevant APIs and with GDB core
in general.

But I have said this many times in the past, and the result is before
your eyes.  So I will now crawl back under my rock.

Re: FYI: minsyms documentation

[Yao Qi]

On 12/23/2011 05:17 AM, Stan Shebs wrote:
> If everybody is able to madly hack away at the code without ever
> consulting the internals manual, then what purpose is it serving
> exactly?  Are newbies learning by reading the manual, or reading the

newbies are learning by manual reading at the beginning, even they can
get more accurate information from comments in code later.

> code?  If the latter, then gdbint.texinfo content might get more
> attention if it was redistributed into 1-2 page blocks at the tops of
> relevant source files.

This sounds good to me, but IMO, we still need gdbint.texinfo to
describe the overall view of components of gdb and their relationships,
which can not be put in source files.

In JDK, all the documentation to a class and its methods are written in
source file, and only one small text file is for package description.
In package description, the overview is given and relationship of these
classes is described.  Most of the doc in details are still in comment
of source file.  I don't think we can copy this approach here, but we
can do something similar here.  Put detailed doc in gdbint.texinfo to
relevant source files, and only leave overview and high-level doc in it.
 In this way, we don't have to pay much effort to keep gdbint.texinfo
synchronized with source, and gdbint.texinfo is still quite useful.

-- 
Yao (齐尧)

Re: FYI: minsyms documentation

[Eli Zaretskii]

> Date: Sat, 24 Dec 2011 11:47:57 +0800
> From: Yao Qi <yao@codesourcery.com>
> 
> newbies are learning by manual reading at the beginning

Then they will learn a lot of incorrect things at the beginning.

> In JDK, all the documentation to a class and its methods are written in
> source file, and only one small text file is for package description.
> In package description, the overview is given and relationship of these
> classes is described.  Most of the doc in details are still in comment
> of source file.  I don't think we can copy this approach here, but we
> can do something similar here.  Put detailed doc in gdbint.texinfo to
> relevant source files, and only leave overview and high-level doc in it.
>  In this way, we don't have to pay much effort to keep gdbint.texinfo
> synchronized with source, and gdbint.texinfo is still quite useful.

Take a look at libiberty and at bfd: they use something very similar.
We could do the same with gdbint.texinfo (I think I proposed that as
well some time ago, but no one was for it).

Re: FYI: minsyms documentation

[Tom Tromey]

>>>>> "Eli" == Eli Zaretskii <eliz@gnu.org> writes:

Stan> Are newbies learning by reading the manual, or reading the code?

Eli> What newbies?  The people who hack at the core features of GDB can be
Eli> counted on fingers of a single hand, and they didn't change in years.

There have been multiple new contributors since I started seriously
working on GDB 3 years ago.

Eli> Again, the current content of that manual can only do a mis-service,
Eli> so redistributing it is wasted effort.

This is not entirely true.  Some of gdbint.texinfo is badly out of date,
but some of it is still relevant.  This is why I have not tried to
delete it.  For example, it is still the only documentation for ui-out
and for cleanups; and, due to licensing, I think that text cannot be
moved into comments in the code without special dispensation from the
FSF.

Tom

Re: FYI: minsyms documentation

[Tom Tromey]

>>>>> "Yao" == Yao Qi <yao@codesourcery.com> writes:

Yao> Put detailed doc in gdbint.texinfo to
Yao> relevant source files, and only leave overview and high-level doc in it.
Yao>  In this way, we don't have to pay much effort to keep gdbint.texinfo
Yao> synchronized with source, and gdbint.texinfo is still quite useful.

Yes, this is exactly what I would like to do.

Tom

Re: FYI: minsyms documentation

[Tom Tromey]

>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:

Joel> So, perhaps the right approach lies in the middle. Only apply
Joel> Tom's approach to parts where it should in fact be an API.

I think one question worth asking is -- what parts of GDB would *not* be
an API?

I think the answer is, or should be, "none".

When I look at GDB, I see a program that has reasonably decent
modularization, though sometimes one must deduce the module boundaries
and rules.  Any given module has its share of API botches, often
involving global variables; but usually with the worst stuff isolated
the oldest and most stable code.

The current GDB has a cleaner GDB inside, struggling to get out.  I'd
like us to spend a bit more effort on chipping away to find it.

This is the spirit in which I wrote the minsym patch series.

I don't see much point in attempting anything like this if the general
opinion of the other maintainers is against it.  I haven't seen enough
replies to consider that there is a consensus, but I will hew to
whatever it is.

Tom

Re: FYI: minsyms documentation

[Joel Brobecker]

> Joel> So, perhaps the right approach lies in the middle. Only apply
> Joel> Tom's approach to parts where it should in fact be an API.
> 
> I think one question worth asking is -- what parts of GDB would *not* be
> an API?
> 
> I think the answer is, or should be, "none".

I would be OK with that. It has the nice property of being simple.
It's similar to me wanting every function to have a description
comment, no matter how useless the comment might appear. That way,
we don't have to try to guess whether documentation is needed or
not.

> I don't see much point in attempting anything like this if the general
> opinion of the other maintainers is against it.  I haven't seen enough
> replies to consider that there is a consensus, but I will hew to
> whatever it is.

I don't have a strong opinion. I think there are pluses and minuses
to both approaches, so it's not a clear decision. We just have to
make a decision. For myself, I have a slight preference for your
suggestion, so count me in.

It is worth mentioning the fact that, when I first started hacking
on GDB, I was appalled at the fact that you could not read a .h file
and get both the API and the associated documentation. To me, it was
incomprehensible that someone would chose to document, and bury,
an API in the .c file. But I also quickly found that this approach
allows you to quickly find the documentation, so I got used to it
and kinda grew to like it.

One of the issues this was supposed to solve, I was told, is that
you can have the same function declared in multiple .h files, which
is a big "ugh", but true I guess.  We could follow Ada's example,
where compilation units have specs and bodies.  The filename is
the same both , except that the .ads extension (spec, equivalent
of .h in C/C++) gets replaced by the .adb extension (body, .c/.cc
in C/C++).  No more ada-tasks.c functions documented in ada-lang.h.
One of my regular fustrations is trying to figure out where functions
declared in values.h or symtab.h are implemented...

GNAT's coding style goes a little further, and requires that all
functions have a declaration before a definition. This is another
debate that we had, with some people liking them, while others
found them to be a maintenance liability. The thing about this
style is that it allows us to have a declaration for every function,
and to document the functions there, presumably at the start of
the unit body. Despite the extra maintenance burden, this has some
advantages as well. But I think it only works well in practice
because there is a compiler switch to enforce that policy - no
function definition without a declaration first.

-- 
Joel

Re: FYI: minsyms documentation

[Eli Zaretskii]

> From: Tom Tromey <tromey@redhat.com>
> Cc: Stan Shebs <stanshebs@earthlink.net>, gdb-patches@sourceware.org
> Date: Mon, 02 Jan 2012 15:07:51 -0700
> 
> >>>>> "Eli" == Eli Zaretskii <eliz@gnu.org> writes:
> 
> Stan> Are newbies learning by reading the manual, or reading the code?
> 
> Eli> What newbies?  The people who hack at the core features of GDB can be
> Eli> counted on fingers of a single hand, and they didn't change in years.
> 
> There have been multiple new contributors since I started seriously
> working on GDB 3 years ago.

I was not talking about contributors.  I was talking about people who
dare hacking at the core features and making structural changes that
cross boundaries of APIs.

> Eli> Again, the current content of that manual can only do a mis-service,
> Eli> so redistributing it is wasted effort.
> 
> This is not entirely true.  Some of gdbint.texinfo is badly out of date,
> but some of it is still relevant.

Without a clear markings which are which, and with large parts of it
badly outdated, it is still a reader-unfriendly document, reading
which runs a very high risk of learning misleading or downright
incorrect information.

Emacs development has a habit that a release requires careful review
of the documentation for inaccuracies, stale or incorrect information,
etc.  We don't have such process; perhaps we should introduce it.

> For example, it is still the only documentation for ui-out and for
> cleanups

I'm willing to bet that even these parts are no longer entirely
accurate or complete, even though at the time they were written, they
were exemplary stuff.

> due to licensing, I think that text cannot be moved into comments in
> the code without special dispensation from the FSF.

This problem can be easily solved: read the text in the manual, close
it, then write the docs in the source files without looking at the
manual, but just at the code.  The result will be different enough
from the original to side-step the copyright issue.  If needed,
another person (e.g., me) can edit the comments to remove any
semblance to the original that sneak in.

Re: FYI: minsyms documentation

[Eli Zaretskii]

> From: Tom Tromey <tromey@redhat.com>
> Cc: <gdb-patches@sourceware.org>
> Date: Mon, 02 Jan 2012 15:08:37 -0700
> 
> >>>>> "Yao" == Yao Qi <yao@codesourcery.com> writes:
> 
> Yao> Put detailed doc in gdbint.texinfo to
> Yao> relevant source files, and only leave overview and high-level doc in it.
> Yao>  In this way, we don't have to pay much effort to keep gdbint.texinfo
> Yao> synchronized with source, and gdbint.texinfo is still quite useful.
> 
> Yes, this is exactly what I would like to do.

I will support that, but please note that the high-level information
also needs to be maintained, because GDB evolves also at that level.

Re: FYI: minsyms documentation

[Pedro Alves]

On 01/03/2012 02:52 AM, Joel Brobecker wrote:
>
> One of the issues this was supposed to solve, I was told, is that
> you can have the same function declared in multiple .h files, which
> is a big "ugh", but true I guess.

Really big "ugh".  That is insane, and we should get rid of such
cases.  Off hand, I know that command.h, and cli/ stuff declare
some of the same functions.  It's not funny to change the
interface/prototype of such a function, update only one of the headers,
and then find at run time you have weird crashes, because some module
is calling the function still using the old prototype, because it pulled
the not-updated header.

-- 
Pedro Alves

Re: FYI: minsyms documentation

[Pedro Alves]

On 12/22/2011 02:34 AM, Tom Tromey wrote:
> I am checking this in on the trunk.
> Today I decided to try to document the minsyms API more or less the way
> I would like APIs to be documented in general.  This patch implements
> that; it move documentation from function definitions to minsyms.h, adds
> an introductory comment about minsyms there as well, and it rearranges
> the header into a more logical order.

+1 from me.

-- 
Pedro Alves

commands.h and cli/cli-decode.h dups (was: Re: FYI: minsyms documentation)

[Pedro Alves]

On 01/03/2012 11:04 AM, Pedro Alves wrote:
> On 01/03/2012 02:52 AM, Joel Brobecker wrote:
>>
>> One of the issues this was supposed to solve, I was told, is that
>> you can have the same function declared in multiple .h files, which
>> is a big "ugh", but true I guess.
>
> Really big "ugh". That is insane, and we should get rid of such
> cases. Off hand, I know that command.h, and cli/ stuff declare
> some of the same functions. It's not funny to change the
> interface/prototype of such a function, update only one of the headers,
> and then find at run time you have weird crashes, because some module
> is calling the function still using the old prototype, because it pulled
> the not-updated header.

Actually looking at that code, it doesn't suffer from that particular
problem, because cli/cli-decode.h includes command.h, so if the
declarations deviated, the compiler would error.

In any case, I've found it confusing to have declarations in both
places, without a clear indication of why we have both places.  I ended
up finding the rationale here:

[rfa:cli] Delete one of the two ``struct cmd_list_element''s
http://sourceware.org/ml/gdb-patches/2002-03/msg00293.html

Andrew wrote:
>  (...) it does also formalize the status quo:
>
>  "command.h": included by any code wanting to create commands.
>
>  "cli-decode.h": included by any code implementing command internals.
>
>  (...)
>  Hmm, if this is ok, the comment at the top of "command.h"
>  should be updated.

But the comment at the top of command.h was never updated.
(command.c is long gone from the sources.)

So this patch does the next step after Andrew's changes.
It eliminates the declaration duplication, by removing
the "public" declarations from cli-decode.h, and the private
interfaces from command.h.  There are perhaps functions that
are "public" today that shouldn't, but we can move the
declarations then if we fix the uses.
It then also adds a couple of comments to the headers to
indicate their purposes.

Comments?

gdb/
2012-01-03  Pedro Alves  <alves.ped@gmail.com>

	* cli/cli-decode.h: Add comments.
	(CMD_LIST_AMBIGUOUS): Moved to command.h
	(add_cmd, add_alias_cmd, add_prefix_cmd, add_abbrev_prefix_cmd)
	(set_cmd_cfunc, set_cmd_sfunc, set_cmd_completer, cmd_cfunc_eq)
	(set_cmd_context, get_cmd_context, lookup_cmd, lookup_cmd_1)
	(deprecate_cmd, deprecated_cmd_warning, lookup_cmd_composition)
	(add_com, add_com_alias, add_info, add_info_alias)
	(complete_on_cmdlist, complete_on_enum, help_list): Remove
	declarations.
	* command.h (CMD_LIST_AMBIGUOUS): Moved here.
	(help_cmd, help_cmd_list): Delete declarations.
---

  gdb/cli/cli-decode.h |   95 ++------------------------------------------------
  gdb/command.h        |   15 +++++---
  2 files changed, 13 insertions(+), 97 deletions(-)

diff --git a/gdb/cli/cli-decode.h b/gdb/cli/cli-decode.h
index e974968..876b169 100644
--- a/gdb/cli/cli-decode.h
+++ b/gdb/cli/cli-decode.h
@@ -19,6 +19,10 @@
  #if !defined (CLI_DECODE_H)
  #define CLI_DECODE_H 1

+/* This file defines the private interfaces for any code implementing
+   command internals.  */
+
+/* Include the public interfaces.  */
  #include "command.h"

  struct re_pattern_buffer;
@@ -210,94 +214,6 @@ struct cmd_list_element
      struct cmd_list_element *alias_chain;
    };

-/* Flag for an ambiguous cmd_list result.  */
-#define CMD_LIST_AMBIGUOUS ((struct cmd_list_element *) -1)
-
-/* API to the manipulation of command lists.  */
-
-extern struct cmd_list_element *add_cmd (char *, enum command_class,
-					 void (*fun) (char *, int), char *,
-					 struct cmd_list_element **);
-
-extern struct cmd_list_element *add_alias_cmd (char *, char *,
-					       enum command_class, int,
-					       struct cmd_list_element **);
-
-extern struct cmd_list_element *add_prefix_cmd (char *, enum command_class,
-						void (*fun) (char *, int),
-						char *,
-						struct cmd_list_element **,
-						char *, int,
-						struct cmd_list_element **);
-
-extern struct cmd_list_element *add_abbrev_prefix_cmd (char *,
-						       enum command_class,
-						       void (*fun) (char *,
-								    int),
-						       char *,
-						       struct cmd_list_element
-						       **, char *, int,
-						       struct cmd_list_element
-						       **);
-
-/* Set the commands corresponding callback.  */
-
-extern void set_cmd_cfunc (struct cmd_list_element *cmd,
-			   void (*cfunc) (char *args, int from_tty));
-
-extern void set_cmd_sfunc (struct cmd_list_element *cmd,
-			   void (*sfunc) (char *args, int from_tty,
-					  struct cmd_list_element * c));
-
-extern void set_cmd_completer (struct cmd_list_element *cmd,
-			       char **(*completer) (struct cmd_list_element *self,
-						    char *text, char *word));
-
-/* HACK: cagney/2002-02-23: Code, mostly in tracepoints.c, grubs
-   around in cmd objects to test the value of the commands sfunc().  */
-extern int cmd_cfunc_eq (struct cmd_list_element *cmd,
-			 void (*cfunc) (char *args, int from_tty));
-
-/* Access to the command's local context.  */
-extern void set_cmd_context (struct cmd_list_element *cmd, void *context);
-extern void *get_cmd_context (struct cmd_list_element *cmd);
-
-extern struct cmd_list_element *lookup_cmd (char **,
-					    struct cmd_list_element *, char *,
-					    int, int);
-
-extern struct cmd_list_element *lookup_cmd_1 (char **,
-					      struct cmd_list_element *,
-					      struct cmd_list_element **,
-					      int);
-
-extern struct cmd_list_element *
-  deprecate_cmd (struct cmd_list_element *, char * );
-
-extern void
-  deprecated_cmd_warning (char **);
-
-extern int
-  lookup_cmd_composition (char *text,
-                        struct cmd_list_element **alias,
-                        struct cmd_list_element **prefix_cmd,
-                        struct cmd_list_element **cmd);
-
-extern struct cmd_list_element *add_com (char *, enum command_class,
-					 void (*fun) (char *, int), char *);
-
-extern struct cmd_list_element *add_com_alias (char *, char *,
-					       enum command_class, int);
-
-extern struct cmd_list_element *add_info (char *, void (*fun) (char *, int),
-					  char *);
-
-extern struct cmd_list_element *add_info_alias (char *, char *, int);
-
-extern char **complete_on_cmdlist (struct cmd_list_element *, char *, char *);
-
-extern char **complete_on_enum (const char *enumlist[], char *, char *);
-
  extern void help_cmd_list (struct cmd_list_element *, enum command_class,
  			   char *, int, struct ui_file *);

@@ -305,9 +221,6 @@ extern void help_cmd_list (struct cmd_list_element *, enum command_class,

  extern void help_cmd (char *, struct ui_file *);

-extern void help_list (struct cmd_list_element *, char *,
-		       enum command_class, struct ui_file *);
-
  extern void apropos_cmd (struct ui_file *, struct cmd_list_element *,
                           struct re_pattern_buffer *, char *);

diff --git a/gdb/command.h b/gdb/command.h
index 04cb751..8112b05 100644
--- a/gdb/command.h
+++ b/gdb/command.h
@@ -1,4 +1,4 @@
-/* Header file for command-reading library command.c.
+/* Header file for command creation.

     Copyright (C) 1986, 1989, 1990, 1991, 1992, 1993, 1994, 1995, 1999, 2000,
     2002, 2004, 2007, 2008, 2009, 2010, 2011 Free Software Foundation, Inc.
@@ -19,6 +19,9 @@
  #if !defined (COMMAND_H)
  #define COMMAND_H 1

+/* This file defines the public interface for any code wanting to
+   create commands.  */
+
  /* Command classes are top-level categories into which commands are
     broken down for "help" purposes.

@@ -106,6 +109,8 @@ struct cmd_list_element;

  /* Forward-declarations of the entry-points of cli/cli-decode.c.  */

+/* API to the manipulation of command lists.  */
+
  extern int valid_user_defined_cmd_name_p (const char *name);

  extern struct cmd_list_element *add_cmd (char *, enum command_class,
@@ -168,6 +173,8 @@ extern void execute_cmd_post_hook (struct cmd_list_element *cmd);
  /* Return the type of the command.  */
  extern enum cmd_types cmd_type (struct cmd_list_element *cmd);

+/* Flag for an ambiguous cmd_list result.  */
+#define CMD_LIST_AMBIGUOUS ((struct cmd_list_element *) -1)

  extern struct cmd_list_element *lookup_cmd (char **,
  					    struct cmd_list_element *, char *,
@@ -207,15 +214,11 @@ extern char **complete_on_cmdlist (struct cmd_list_element *,
  extern char **complete_on_enum (const char *enumlist[],
  				char *, char *);

-extern void help_cmd (char *, struct ui_file *);
+/* Functions that implement commands about CLI commands.  */

  extern void help_list (struct cmd_list_element *, char *,
  		       enum command_class, struct ui_file *);

-extern void help_cmd_list (struct cmd_list_element *,
-			   enum command_class,
-			   char *, int, struct ui_file *);
-
  /* Method for show a set/show variable's VALUE on FILE.  If this
     method isn't supplied deprecated_show_value_hack() is called (which
     is not good).  */

Re: commands.h and cli/cli-decode.h dups

[Tom Tromey]

>>>>> "Pedro" == Pedro Alves <alves.ped@gmail.com> writes:

Pedro> So this patch does the next step after Andrew's changes.
Pedro> It eliminates the declaration duplication, by removing
Pedro> the "public" declarations from cli-decode.h, and the private
Pedro> interfaces from command.h.  There are perhaps functions that
Pedro> are "public" today that shouldn't, but we can move the
Pedro> declarations then if we fix the uses.
Pedro> It then also adds a couple of comments to the headers to
Pedro> indicate their purposes.

Pedro> Comments?

I am totally in favor of it.
I think duplication of this sort is a "worst practice".

Tom

Re: commands.h and cli/cli-decode.h dups

[Joel Brobecker]

> I am totally in favor of it.
> I think duplication of this sort is a "worst practice".

+1

-- 
Joel

Re: commands.h and cli/cli-decode.h dups

[Pedro Alves]

Thanks guys.  Now applied.

-- 
Pedro Alves

Re: FYI: minsyms documentation

[Michael Eager]

On 12/21/2011 06:34 PM, Tom Tromey wrote:
> I am checking this in on the trunk.
>
> Today I decided to try to document the minsyms API more or less the way
> I would like APIs to be documented in general.  This patch implements
> that; it move documentation from function definitions to minsyms.h, adds
> an introductory comment about minsyms there as well, and it rearranges
> the header into a more logical order.

I realize that I'm late to this discussion, but I dislike this approach
and think that it will make maintaining GDB more difficult and makes
using and maintaining the documentation more difficult.

It requires looking in two places, the .c and the .h at the same
time.  When I'm debugging something, the .c is in the debugger window
and if there are any useful comments I see them.  This change requires
me to open the .h in a different editor window and search for the
function name.  Most of the time I'm not going to do this, since almost
all the time, the comments don't say very much.  This means that when
there is a comment which might explain what to me is an unexpected
behavior, I'm likely to see it only much later, if at all, when it
occurs to me that there might be something in the .h file which is
relevant.

Best practice, comments are best kept close to the code that they
refer to.  Maintainers are likely to update comments if they are
next to the modified code, and much less likely if the comments are
somewhere else.  It doesn't matter if that somewhere else is in a .h
file, or in the .texi files, or in some other area.  There are actually
decades of experimental evidence in favor of keeping detailed comments
close to the relevant code, rather than removing them to a different
location.

It would be nice to see GDB documentation improve and documenting
the APIs would be great.  I see this as providing high level concept
documentation which explains how a particular API works, why it exists,
and how it interacts with other APIs.  Minimal documentation of each
function, without context, is not this.  As far as I can tell, there
minsyms.h contains one very short paragraph which provides a bit of
context and there is no general overview.

To use an analogy, this is like giving you the description of a
spark plug, a fuel pump, and a crankshaft, and expecting you to figure
out how these interact to make a car engine turn.

-- 
Michael Eager	 eager@eagercon.com
1960 Park Blvd., Palo Alto, CA 94306  650-325-8077