Re: [RFA 01/42] Use new and delete for buildsym_compunit

[Keith Seitz <keiths at redhat dot com>]

Hi, Tom,

I've got one general question:

On 05/22/2018 09:58 PM, Tom Tromey wrote:
> This changes buildsym_compunit to use new and delete.
> 
> diff --git a/gdb/buildsym.c b/gdb/buildsym.c
> index 5d38cb250f..9863e4ea1e 100644
> --- a/gdb/buildsym.c
> +++ b/gdb/buildsym.c
> @@ -107,24 +136,24 @@ struct buildsym_compunit
>       Files are added to the front of the list.
>       This is important mostly for the language determination hacks we use,
>       which iterate over previously added files.  */
> -  struct subfile *subfiles;
> +  struct subfile *subfiles = nullptr;
>  
>    /* The subfile of the main source file.  */
> -  struct subfile *main_subfile;
> +  struct subfile *main_subfile = nullptr;
>  
>    /* E.g., DW_AT_comp_dir if DWARF.  Space for this is malloc'd.  */
>    char *comp_dir;
>  
>    /* Space for this is not malloc'd, and is assumed to have at least
>       the same lifetime as objfile.  */
> -  const char *producer;
> +  const char *producer = nullptr;
>  
>    /* Space for this is not malloc'd, and is assumed to have at least
>       the same lifetime as objfile.  */
> -  const char *debugformat;
> +  const char *debugformat = nullptr;
>  
>    /* The compunit we are building.  */
> -  struct compunit_symtab *compunit_symtab;
> +  struct compunit_symtab *compunit_symtab = nullptr;
>  
>    /* Language of this compunit_symtab.  */
>    enum language language;

By the end of the series, this struct declaration is moved to buildsym.h and a bunch of new, private data is added. All those data members are named "m_XYZ" whereas the above variable names are never receive the "m_" treatment.

Since these variables will later become private, should they not also (some time) be renamed?
[I think this should/could be a follow-on patch -- no changes needed here.]

Keith

PS:

diff --git a/gdb/buildsym.c b/gdb/buildsym.c
index 0c7efa3d51..32f9b69331 100644
--- a/gdb/buildsym.c
+++ b/gdb/buildsym.c
@@ -70,15 +70,15 @@ buildsym_compunit::buildsym_compunit (struct objfile *objfile_,
 				      const char *comp_dir_,
 				      enum language language_,
 				      CORE_ADDR last_addr)
-  : objfile (objfile_),
+  : m_objfile (objfile_),
     m_last_source_file (name == nullptr ? nullptr : xstrdup (name)),
-    comp_dir (comp_dir_ == nullptr ? nullptr : xstrdup (comp_dir_)),
-    language (language_),
+    m_comp_dir (comp_dir_ == nullptr ? nullptr : xstrdup (comp_dir_)),
+    m_language (language_),
     m_last_source_start_addr (last_addr)
 {
   /* Allocate the compunit symtab now.  The caller needs it to allocate
      non-primary symtabs.  It is also needed by get_macro_table.  */
-  compunit_symtab = allocate_compunit_symtab (objfile, name);
+  m_compunit_symtab = allocate_compunit_symtab (m_objfile, name);
 
   /* Build the subfile for NAME (the main source file) so that we can record
      a pointer to it for later.
@@ -89,7 +89,7 @@ buildsym_compunit::buildsym_compunit (struct objfile *objfile_,
   start_subfile (name);
   /* Save this so that we don't have to go looking for it at the end
      of the subfiles list.  */
-  main_subfile = m_current_subfile;
+  m_main_subfile = m_current_subfile;
 }
 
 buildsym_compunit::~buildsym_compunit ()
@@ -99,7 +99,7 @@ buildsym_compunit::~buildsym_compunit ()
   if (m_pending_macros != nullptr)
     free_macro_table (m_pending_macros);
 
-  for (subfile = subfiles;
+  for (subfile = m_subfiles;
        subfile != NULL;
        subfile = nextsub)
     {
@@ -128,9 +128,9 @@ struct macro_table *
 buildsym_compunit::get_macro_table ()
 {
   if (m_pending_macros == nullptr)
-    m_pending_macros = new_macro_table (&objfile->per_bfd->storage_obstack,
-					objfile->per_bfd->macro_cache,
-					compunit_symtab);
+    m_pending_macros = new_macro_table (&m_objfile->per_bfd->storage_obstack,
+					m_objfile->per_bfd->macro_cache,
+					m_compunit_symtab);
   return m_pending_macros;
 }
 
@@ -237,34 +237,34 @@ buildsym_compunit::finish_block_internal
      CORE_ADDR start, CORE_ADDR end,
      int is_global, int expandable)
 {
-  struct gdbarch *gdbarch = get_objfile_arch (objfile);
+  struct gdbarch *gdbarch = get_objfile_arch (m_objfile);
   struct pending *next, *next1;
   struct block *block;
   struct pending_block *pblock;
   struct pending_block *opblock;
 
   block = (is_global
-	   ? allocate_global_block (&objfile->objfile_obstack)
-	   : allocate_block (&objfile->objfile_obstack));
+	   ? allocate_global_block (&m_objfile->objfile_obstack)
+	   : allocate_block (&m_objfile->objfile_obstack));
 
   if (symbol)
     {
       BLOCK_DICT (block)
-	= dict_create_linear (&objfile->objfile_obstack,
-			      language, *listhead);
+	= dict_create_linear (&m_objfile->objfile_obstack,
+			      m_language, *listhead);
     }
   else
     {
       if (expandable)
 	{
-	  BLOCK_DICT (block) = dict_create_hashed_expandable (language);
+	  BLOCK_DICT (block) = dict_create_hashed_expandable (m_language);
 	  dict_add_pending (BLOCK_DICT (block), *listhead);
 	}
       else
 	{
 	  BLOCK_DICT (block) =
-	    dict_create_hashed (&objfile->objfile_obstack,
-				language, *listhead);
+	    dict_create_hashed (&m_objfile->objfile_obstack,
+				m_language, *listhead);
 	}
     }
 
@@ -325,7 +325,7 @@ buildsym_compunit::finish_block_internal
     }
 
   if (static_link != NULL)
-    objfile_register_static_link (objfile, block, static_link);
+    objfile_register_static_link (m_objfile, block, static_link);
 
   /* Now free the links of the list, and empty the list.  */
 
@@ -407,7 +407,7 @@ buildsym_compunit::finish_block_internal
 		   (is_global
 		    ? m_global_using_directives
 		    : m_local_using_directives),
-		   &objfile->objfile_obstack);
+		   &m_objfile->objfile_obstack);
   if (is_global)
     m_global_using_directives = NULL;
   else
@@ -470,7 +470,7 @@ buildsym_compunit::make_blockvector ()
     }
 
   blockvector = (struct blockvector *)
-    obstack_alloc (&objfile->objfile_obstack,
+    obstack_alloc (&m_objfile->objfile_obstack,
 		   (sizeof (struct blockvector)
 		    + (i - 1) * sizeof (struct block *)));
 
@@ -492,7 +492,7 @@ buildsym_compunit::make_blockvector ()
      blockvector.  */
   if (m_pending_addrmap && m_pending_addrmap_interesting)
     BLOCKVECTOR_MAP (blockvector)
-      = addrmap_create_fixed (m_pending_addrmap, &objfile->objfile_obstack);
+      = addrmap_create_fixed (m_pending_addrmap, &m_objfile->objfile_obstack);
   else
     BLOCKVECTOR_MAP (blockvector) = 0;
 
@@ -531,11 +531,11 @@ buildsym_compunit::start_subfile (const char *name)
   const char *subfile_dirname;
   struct subfile *subfile;
 
-  subfile_dirname = comp_dir.get ();
+  subfile_dirname = m_comp_dir.get ();
 
   /* See if this subfile is already registered.  */
 
-  for (subfile = subfiles; subfile; subfile = subfile->next)
+  for (subfile = m_subfiles; subfile; subfile = subfile->next)
     {
       char *subfile_name;
 
@@ -566,8 +566,8 @@ buildsym_compunit::start_subfile (const char *name)
   memset (subfile, 0, sizeof (struct subfile));
   subfile->buildsym_compunit = this;
 
-  subfile->next = subfiles;
-  subfiles = subfile;
+  subfile->next = m_subfiles;
+  m_subfiles = subfile;
 
   m_current_subfile = subfile;
 
@@ -605,7 +605,7 @@ buildsym_compunit::start_subfile (const char *name)
       enum language sublang = deduce_language_from_filename (subfile->name);
 
       if (sublang == language_cplus || sublang == language_fortran)
-	for (s = subfiles; s != NULL; s = s->next)
+	for (s = m_subfiles; s != NULL; s = s->next)
 	  if (s->language == language_c)
 	    s->language = sublang;
     }
@@ -648,11 +648,11 @@ buildsym_compunit::patch_subfile_names (struct subfile *subfile,
 					const char *name)
 {
   if (subfile != NULL
-      && comp_dir == NULL
+      && m_comp_dir == NULL
       && subfile->name != NULL
       && IS_DIR_SEPARATOR (subfile->name[strlen (subfile->name) - 1]))
     {
-      comp_dir.reset (subfile->name);
+      m_comp_dir.reset (subfile->name);
       subfile->name = xstrdup (name);
       set_last_source_file (name);
 
@@ -872,7 +872,7 @@ buildsym_compunit::watch_main_source_file_lossage ()
   struct subfile *mainsub, *subfile;
 
   /* Get the main source file.  */
-  mainsub = main_subfile;
+  mainsub = m_main_subfile;
 
   /* If the main source file doesn't have any line number or symbol
      info, look for an alias in another subfile.  */
@@ -887,7 +887,7 @@ buildsym_compunit::watch_main_source_file_lossage ()
       struct subfile *prev_mainsub_alias = NULL;
 
       prevsub = NULL;
-      for (subfile = subfiles;
+      for (subfile = m_subfiles;
 	   subfile != NULL;
 	   subfile = subfile->next)
 	{
@@ -915,7 +915,7 @@ buildsym_compunit::watch_main_source_file_lossage ()
 	  mainsub->symtab = mainsub_alias->symtab;
 
 	  if (prev_mainsub_alias == NULL)
-	    subfiles = mainsub_alias->next;
+	    m_subfiles = mainsub_alias->next;
 	  else
 	    prev_mainsub_alias->next = mainsub_alias->next;
 	  xfree (mainsub_alias->name);
@@ -968,7 +968,7 @@ buildsym_compunit::end_symtab_get_static_block (CORE_ADDR end_addr,
   /* Reordered executables may have out of order pending blocks; if
      OBJF_REORDERED is true, then sort the pending blocks.  */
 
-  if ((objfile->flags & OBJF_REORDERED) && m_pending_blocks)
+  if ((m_objfile->flags & OBJF_REORDERED) && m_pending_blocks)
     {
       struct pending_block *pb;
 
@@ -1001,8 +1001,8 @@ buildsym_compunit::end_symtab_get_static_block (CORE_ADDR end_addr,
      are no-ops.  FIXME: Is this handled right in case of QUIT?  Can
      we make this cleaner?  */
 
-  cleanup_undefined_stabs_types (objfile);
-  finish_global_stabs (objfile);
+  cleanup_undefined_stabs_types (m_objfile);
+  finish_global_stabs (m_objfile);
 
   if (!required
       && m_pending_blocks == NULL
@@ -1032,14 +1032,14 @@ struct compunit_symtab *
 buildsym_compunit::end_symtab_with_blockvector (struct block *static_block,
 						int section, int expandable)
 {
-  struct compunit_symtab *cu = compunit_symtab;
+  struct compunit_symtab *cu = m_compunit_symtab;
   struct symtab *symtab;
   struct blockvector *blockvector;
   struct subfile *subfile;
   CORE_ADDR end_addr;
 
   gdb_assert (static_block != NULL);
-  gdb_assert (subfiles != NULL);
+  gdb_assert (m_subfiles != NULL);
 
   end_addr = BLOCK_END (static_block);
 
@@ -1051,8 +1051,8 @@ buildsym_compunit::end_symtab_with_blockvector (struct block *static_block,
 
   /* Read the line table if it has to be read separately.
      This is only used by xcoffread.c.  */
-  if (objfile->sf->sym_read_linetable != NULL)
-    objfile->sf->sym_read_linetable (objfile);
+  if (m_objfile->sf->sym_read_linetable != NULL)
+    m_objfile->sf->sym_read_linetable (m_objfile);
 
   /* Handle the case where the debug info specifies a different path
      for the main source file.  It can cause us to lose track of its
@@ -1062,7 +1062,7 @@ buildsym_compunit::end_symtab_with_blockvector (struct block *static_block,
   /* Now create the symtab objects proper, if not already done,
      one for each subfile.  */
 
-  for (subfile = subfiles;
+  for (subfile = m_subfiles;
        subfile != NULL;
        subfile = subfile->next)
     {
@@ -1076,7 +1076,7 @@ buildsym_compunit::end_symtab_with_blockvector (struct block *static_block,
 	  /* Like the pending blocks, the line table may be
 	     scrambled in reordered executables.  Sort it if
 	     OBJF_REORDERED is true.  */
-	  if (objfile->flags & OBJF_REORDERED)
+	  if (m_objfile->flags & OBJF_REORDERED)
 	    qsort (subfile->line_vector->item,
 		   subfile->line_vector->nitems,
 		   sizeof (struct linetable_entry), compare_line_numbers);
@@ -1093,7 +1093,7 @@ buildsym_compunit::end_symtab_with_blockvector (struct block *static_block,
 	{
 	  /* Reallocate the line table on the symbol obstack.  */
 	  SYMTAB_LINETABLE (symtab) = (struct linetable *)
-	    obstack_alloc (&objfile->objfile_obstack, linetablesize);
+	    obstack_alloc (&m_objfile->objfile_obstack, linetablesize);
 	  memcpy (SYMTAB_LINETABLE (symtab), subfile->line_vector,
 		  linetablesize);
 	}
@@ -1115,7 +1115,7 @@ buildsym_compunit::end_symtab_with_blockvector (struct block *static_block,
   {
     struct symtab *main_symtab, *prev_symtab;
 
-    main_symtab = main_subfile->symtab;
+    main_symtab = m_main_subfile->symtab;
     prev_symtab = NULL;
     ALL_COMPUNIT_FILETABS (cu, symtab)
       {
@@ -1136,20 +1136,20 @@ buildsym_compunit::end_symtab_with_blockvector (struct block *static_block,
 
   /* Fill out the compunit symtab.  */
 
-  if (comp_dir != NULL)
+  if (m_comp_dir != NULL)
     {
       /* Reallocate the dirname on the symbol obstack.  */
-      const char *comp_dir = this->comp_dir.get ();
+      const char *comp_dir = m_comp_dir.get ();
       COMPUNIT_DIRNAME (cu)
-	= (const char *) obstack_copy0 (&objfile->objfile_obstack,
+	= (const char *) obstack_copy0 (&m_objfile->objfile_obstack,
 					comp_dir, strlen (comp_dir));
     }
 
   /* Save the debug format string (if any) in the symtab.  */
-  COMPUNIT_DEBUGFORMAT (cu) = debugformat;
+  COMPUNIT_DEBUGFORMAT (cu) = m_debugformat;
 
   /* Similarly for the producer.  */
-  COMPUNIT_PRODUCER (cu) = producer;
+  COMPUNIT_PRODUCER (cu) = m_producer;
 
   COMPUNIT_BLOCKVECTOR (cu) = blockvector;
   {
@@ -1298,7 +1298,7 @@ set_missing_symtab (struct pending *pending_list,
 void
 buildsym_compunit::augment_type_symtab ()
 {
-  struct compunit_symtab *cust = compunit_symtab;
+  struct compunit_symtab *cust = m_compunit_symtab;
   const struct blockvector *blockvector = COMPUNIT_BLOCKVECTOR (cust);
 
   if (!m_context_stack.empty ())
diff --git a/gdb/buildsym.h b/gdb/buildsym.h
index d90faff903..41d1c61ba2 100644
--- a/gdb/buildsym.h
+++ b/gdb/buildsym.h
@@ -124,11 +124,11 @@ struct buildsym_compunit
   buildsym_compunit (struct objfile *objfile_, const char *name,
 		     const char *comp_dir_, enum language language_,
 		     CORE_ADDR last_addr, struct compunit_symtab *cust)
-    : objfile (objfile_),
+    : m_objfile (objfile_),
       m_last_source_file (name == nullptr ? nullptr : xstrdup (name)),
-      comp_dir (comp_dir_ == nullptr ? nullptr : xstrdup (comp_dir_)),
-      compunit_symtab (cust),
-      language (language_),
+      m_comp_dir (comp_dir_ == nullptr ? nullptr : xstrdup (comp_dir_)),
+      m_compunit_symtab (cust),
+      m_language (language_),
       m_last_source_start_addr (last_addr)
   {
   }
@@ -185,7 +185,7 @@ struct buildsym_compunit
 
   struct compunit_symtab *get_compunit_symtab ()
   {
-    return compunit_symtab;
+    return m_compunit_symtab;
   }
 
   void set_last_source_start_addr (CORE_ADDR addr)
@@ -252,12 +252,12 @@ struct buildsym_compunit
 
   void record_debugformat (const char *format)
   {
-    debugformat = format;
+    m_debugformat = format;
   }
 
   void record_producer (const char *producer)
   {
-    this->producer = producer;
+    m_producer = producer;
   }
 
   struct context_stack *push_context (int desc, CORE_ADDR valu);
@@ -296,16 +296,16 @@ private:
       (struct block *static_block, int section, int expandable);
 
   /* The objfile we're reading debug info from.  */
-  struct objfile *objfile;
+  struct objfile *m_objfile;
 
   /* List of subfiles (source files).
      Files are added to the front of the list.
      This is important mostly for the language determination hacks we use,
      which iterate over previously added files.  */
-  struct subfile *subfiles = nullptr;
+  struct subfile *m_subfiles = nullptr;
 
   /* The subfile of the main source file.  */
-  struct subfile *main_subfile = nullptr;
+  struct subfile *m_main_subfile = nullptr;
 
   /* Name of source file whose symbol data we are now processing.  This
      comes from a symbol of type N_SO for stabs.  For Dwarf it comes
@@ -314,21 +314,21 @@ private:
   gdb::unique_xmalloc_ptr<char> m_last_source_file;
 
   /* E.g., DW_AT_comp_dir if DWARF.  Space for this is malloc'd.  */
-  gdb::unique_xmalloc_ptr<char> comp_dir;
+  gdb::unique_xmalloc_ptr<char> m_comp_dir;
 
   /* Space for this is not malloc'd, and is assumed to have at least
      the same lifetime as objfile.  */
-  const char *producer = nullptr;
+  const char *m_producer = nullptr;
 
   /* Space for this is not malloc'd, and is assumed to have at least
      the same lifetime as objfile.  */
-  const char *debugformat = nullptr;
+  const char *m_debugformat = nullptr;
 
   /* The compunit we are building.  */
-  struct compunit_symtab *compunit_symtab = nullptr;
+  struct compunit_symtab *m_compunit_symtab = nullptr;
 
   /* Language of this compunit_symtab.  */
-  enum language language;
+  enum language m_language;
 
   /* The macro table for the compilation unit whose symbols we're
      currently reading.  */