[RFA,10/42] Move some code from buildsym to stabsread

Message ID 20180523045851.11660-11-tom@tromey.com
State New
Headers show
Series
  • Remove globals from buildsym
Related show

Commit Message

Tom Tromey May 23, 2018, 4:58 a.m.
A few things that currently reside in buildsym.c turn out to be
specific to the stabs reader.  This patch moves these from
buildsym.[ch] to stabsread.[ch].

gdb/ChangeLog
2018-05-22  Tom Tromey  <tom@tromey.com>

	* stabsread.h (HASHSIZE, hashname, symnum, next_symbol_text)
	(next_symbol_text_func): Move from buildsym.h.
	* stabsread.c (hashname): Move from buildsym.c.
	* buildsym.h (HASHSIZE, symnum, next_symbol_text)
	(next_symbol_text_func, hashname): Move to stabsread.h.
	* buildsym.c: Don't include bcache.h
	(hashname): Move to stasbread.c.
---
 gdb/ChangeLog   | 10 ++++++++++
 gdb/buildsym.c  | 10 ----------
 gdb/buildsym.h  | 15 ---------------
 gdb/stabsread.c | 13 +++++++++++++
 gdb/stabsread.h | 15 +++++++++++++++
 5 files changed, 38 insertions(+), 25 deletions(-)

-- 
2.13.6

Comments

Keith Seitz July 5, 2018, 7:16 p.m. | #1
I just have one comment:

On 05/22/2018 09:58 PM, Tom Tromey wrote:
> diff --git a/gdb/stabsread.c b/gdb/stabsread.c

> index 4cfefaa3e0..13663106dc 100644

> --- a/gdb/stabsread.c

> +++ b/gdb/stabsread.c

> @@ -44,6 +44,7 @@

>  #include "target-float.h"

>  #include "cp-abi.h"

>  #include "cp-support.h"

> +#include "bcache.h"

>  #include <ctype.h>

>  

>  /* Ask stabsread.h to define the vars it normally declares `extern'.  */

> @@ -4839,6 +4840,18 @@ find_name_end (const char *name)

>      }

>  }

>  

> +

> +

> +/* Compute a small integer hash code for the given name.  */

> +

> +int

> +hashname (const char *name)

> +{

> +  return hash (name, strlen (name)) % HASHSIZE;

> +}

> +

> +

> +

>  /* Initializer for this module.  */

>  

>  void


Seems some superfluous newlines got pasted in here?
Please use "See stabsread.h" and move the comment into the header.

Okay, that's actually two comments, but one is so trivial, I didn't count it.

Keith
Simon Marchi July 8, 2018, 4:35 p.m. | #2
On 2018-07-05 03:16 PM, Keith Seitz wrote:
> I just have one comment:

> 

> On 05/22/2018 09:58 PM, Tom Tromey wrote:

>> diff --git a/gdb/stabsread.c b/gdb/stabsread.c

>> index 4cfefaa3e0..13663106dc 100644

>> --- a/gdb/stabsread.c

>> +++ b/gdb/stabsread.c

>> @@ -44,6 +44,7 @@

>>  #include "target-float.h"

>>  #include "cp-abi.h"

>>  #include "cp-support.h"

>> +#include "bcache.h"

>>  #include <ctype.h>

>>  

>>  /* Ask stabsread.h to define the vars it normally declares `extern'.  */

>> @@ -4839,6 +4840,18 @@ find_name_end (const char *name)

>>      }

>>  }

>>  

>> +

>> +

>> +/* Compute a small integer hash code for the given name.  */

>> +

>> +int

>> +hashname (const char *name)

>> +{

>> +  return hash (name, strlen (name)) % HASHSIZE;

>> +}

>> +

>> +

>> +

>>  /* Initializer for this module.  */

>>  

>>  void

> 

> Seems some superfluous newlines got pasted in here?


The newline in the middle is actually a form feed character, as recommended in the
GNU coding standards [1]:

"Please use formfeed characters (control-L) to divide the program into pages at logical
places (but not within a function). It does not matter just how long the pages are,
since they do not have to fit on a printed page. The formfeeds should appear alone on
lines by themselves."

I'd be really curious to know if anybody cares about them today.  I know in some
editors you can jump from on to the other.  In my editor, they just appear as the
little "special character" box with the ascii/unicode in it.

[1] https://www.gnu.org/prep/standards/html_node/Formatting.html

> Please use "See stabsread.h" and move the comment into the header.

> 

> Okay, that's actually two comments, but one is so trivial, I didn't count it.

> 

> Keith

> 


LGTM with Keith's comment about the comment addressed.

Simon
Tom Tromey July 8, 2018, 4:37 p.m. | #3
>>>>> "Keith" == Keith Seitz <keiths@redhat.com> writes:


Keith> Seems some superfluous newlines got pasted in here?

I usually put lines around the ^Ls, but it doesn't matter to me so I
removed them.

Keith> Please use "See stabsread.h" and move the comment into the header.

I did this.

Tom
Tom Tromey July 8, 2018, 4:59 p.m. | #4
>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:


Simon> The newline in the middle is actually a form feed character, as recommended in the
Simon> GNU coding standards [1]:

Hah, it didn't even occur to me that these wouldn't have shown up, but
that's probably what happened.

Simon> I'd be really curious to know if anybody cares about them today.  I know in some
Simon> editors you can jump from on to the other.  In my editor, they just appear as the
Simon> little "special character" box with the ascii/unicode in it.

I use them as indicators of "this group of functions belongs together",
but I don't actually use the page navigation functions in Emacs.  Also
there's a historical / habitual component to adding them; it's long been
part of the GNU style.

Maybe having more and smaller files would be a better approach to what I
use them for.  Though of course that's not without its costs.

Tom

Patch

diff --git a/gdb/buildsym.c b/gdb/buildsym.c
index 85b450383e..0e69865886 100644
--- a/gdb/buildsym.c
+++ b/gdb/buildsym.c
@@ -74,7 +74,6 @@ 
 #include "gdbtypes.h"
 #include "complaints.h"
 #include "expression.h"		/* For "enum exp_opcode" used by...  */
-#include "bcache.h"
 #include "filenames.h"		/* For DOSish file names.  */
 #include "macrotab.h"
 #include "demangle.h"		/* Needed by SYMBOL_INIT_DEMANGLED_NAME.  */
@@ -1675,15 +1674,6 @@  pop_context (void)
 
 
 
-/* Compute a small integer hash code for the given name.  */
-
-int
-hashname (const char *name)
-{
-    return (hash(name,strlen(name)) % HASHSIZE);
-}
-
-
 void
 record_debugformat (const char *format)
 {
diff --git a/gdb/buildsym.h b/gdb/buildsym.h
index 191db8ca4f..0b19c39305 100644
--- a/gdb/buildsym.h
+++ b/gdb/buildsym.h
@@ -46,9 +46,6 @@  struct dynamic_prop;
 #define	EXTERN extern
 #endif
 
-#define HASHSIZE 127		/* Size of things hashed via
-				   hashname().  */
-
 /* The list of sub-source-files within the current individual
    compilation.  Each file gets its own symtab with its own linetable
    and associated info, but they all share one blockvector.  */
@@ -74,10 +71,6 @@  EXTERN struct subfile *current_subfile;
 
 EXTERN unsigned char processing_gcc_compilation;
 
-/* Count symbols as they are processed, for error messages.  */
-
-EXTERN unsigned int symnum;
-
 /* Record the symbols defined for each context in a list.  We don't
    create a struct block for the context until we know how long to
    make it.  */
@@ -173,12 +166,6 @@  typedef void (record_line_ftype) (struct subfile *subfile, int line,
 
 
 
-#define next_symbol_text(objfile) (*next_symbol_text_func)(objfile)
-
-/* Function to invoke get the next symbol.  Return the symbol name.  */
-
-EXTERN const char *(*next_symbol_text_func) (struct objfile *);
-
 extern void add_symbol_to_list (struct symbol *symbol,
 				struct pending **listhead);
 
@@ -251,8 +238,6 @@  extern struct compunit_symtab *start_symtab (struct objfile *objfile,
 extern void restart_symtab (struct compunit_symtab *cust,
 			    const char *name, CORE_ADDR start_addr);
 
-extern int hashname (const char *name);
-
 extern void free_pending_blocks (void);
 
 /* Record the name of the debug format in the current pending symbol
diff --git a/gdb/stabsread.c b/gdb/stabsread.c
index 4cfefaa3e0..13663106dc 100644
--- a/gdb/stabsread.c
+++ b/gdb/stabsread.c
@@ -44,6 +44,7 @@ 
 #include "target-float.h"
 #include "cp-abi.h"
 #include "cp-support.h"
+#include "bcache.h"
 #include <ctype.h>
 
 /* Ask stabsread.h to define the vars it normally declares `extern'.  */
@@ -4839,6 +4840,18 @@  find_name_end (const char *name)
     }
 }
 
+
+
+/* Compute a small integer hash code for the given name.  */
+
+int
+hashname (const char *name)
+{
+  return hash (name, strlen (name)) % HASHSIZE;
+}
+
+
+
 /* Initializer for this module.  */
 
 void
diff --git a/gdb/stabsread.h b/gdb/stabsread.h
index d24b25dc72..7958990112 100644
--- a/gdb/stabsread.h
+++ b/gdb/stabsread.h
@@ -31,6 +31,21 @@  enum language;
 #define	EXTERN extern
 #endif
 
+#define HASHSIZE 127		/* Size of things hashed via
+				   hashname().  */
+
+extern int hashname (const char *name);
+
+/* Count symbols as they are processed, for error messages.  */
+
+EXTERN unsigned int symnum;
+
+#define next_symbol_text(objfile) (*next_symbol_text_func)(objfile)
+
+/* Function to invoke get the next symbol.  Return the symbol name.  */
+
+EXTERN const char *(*next_symbol_text_func) (struct objfile *);
+
 /* Hash table of global symbols whose values are not known yet.
    They are chained thru the SYMBOL_VALUE_CHAIN, since we don't
    have the correct data for that slot yet.