From patchwork Thu Jul 5 18:49:56 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Keith Seitz X-Patchwork-Id: 28245 Received: (qmail 124933 invoked by alias); 5 Jul 2018 18:50:01 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 124549 invoked by uid 89); 5 Jul 2018 18:50:01 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-25.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_LAZY_DOMAIN_SECURITY, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=HTo:U*tom, xfree X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 05 Jul 2018 18:49:58 +0000 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 65EE0C04BD3B; Thu, 5 Jul 2018 18:49:57 +0000 (UTC) Received: from theo.uglyboxes.com (ovpn04.gateway.prod.ext.phx2.redhat.com [10.5.9.4]) by smtp.corp.redhat.com (Postfix) with ESMTPS id DFB115C265; Thu, 5 Jul 2018 18:49:56 +0000 (UTC) Subject: Re: [RFA 01/42] Use new and delete for buildsym_compunit To: Tom Tromey , gdb-patches@sourceware.org References: <20180523045851.11660-1-tom@tromey.com> <20180523045851.11660-2-tom@tromey.com> From: Keith Seitz Message-ID: <8a3cd7f5-f328-55c8-2762-3aa315e339c1@redhat.com> Date: Thu, 5 Jul 2018 11:49:56 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.2 MIME-Version: 1.0 In-Reply-To: <20180523045851.11660-2-tom@tromey.com> X-IsSubscribed: yes 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 m_last_source_file; /* E.g., DW_AT_comp_dir if DWARF. Space for this is malloc'd. */ - gdb::unique_xmalloc_ptr comp_dir; + gdb::unique_xmalloc_ptr 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. */