From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 4722 invoked by alias); 1 Jun 2018 19:19:41 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 4613 invoked by uid 89); 1 Jun 2018 19:19:32 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-23.4 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_LAZY_DOMAIN_SECURITY,KAM_STOCKGEN,SEM_URIRED,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy= 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; Fri, 01 Jun 2018 19:19:31 +0000 Received: from smtp.corp.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.24]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id DAB21AE683; Fri, 1 Jun 2018 19:19:29 +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 A214B308BDA7; Fri, 1 Jun 2018 19:19:29 +0000 (UTC) Subject: Re: [RFA 2/6] Change representation of psymbol to flush out accessors To: Tom Tromey , gdb-patches@sourceware.org References: <20180503223621.22544-1-tom@tromey.com> <20180503223621.22544-3-tom@tromey.com> From: Keith Seitz Message-ID: <15d0da9f-93fa-5d23-ce4e-43d5befe48e7@redhat.com> Date: Fri, 01 Jun 2018 19:19:00 -0000 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: <20180503223621.22544-3-tom@tromey.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2018-06/txt/msg00031.txt.bz2 On 05/03/2018 03:36 PM, Tom Tromey wrote: > ChangeLog > 2018-05-03 Tom Tromey > > * dwarf-index-write.c (write_psymbols, debug_names::insert) > (debug_names::write_psymbols): Update. > * psympriv.h (struct partial_symbol) : Rename from > 'ginfo'. > (PSYMBOL_VALUE, PSYMBOL_VALUE_ADDRESS, PSYMBOL_LANGUAGE) > (PSYMBOL_SECTION, PSYMBOL_OBJ_SECTION, PSYMBOL_SET_LANGUAGE) > (PSYMBOL_SET_NAMES, PSYMBOL_LINKAGE_NAME, PSYMBOL_DEMANGLED_NAME) > (PSYMBOL_SEARCH_NAME, PSYMBOL_MATCHES_SEARCH_NAME): New macros. > * psymtab.c (find_pc_sect_psymtab_closer, find_pc_sect_psymtab) > (find_pc_sect_psymbol, fixup_psymbol_section) > (match_partial_symbol, lookup_partial_symbol, relocate_psymtabs) > (print_partial_symbols, recursively_search_psymtabs) > (compare_psymbols, psymbol_hash, psymbol_compare) > (add_psymbol_to_bcache, maintenance_check_psymtabs) > (psymbol_name_matches, psym_fill_psymbol_map): Update. One trivial comment, otherwise IANAM, but LGTM. Keith > diff --git a/gdb/psymtab.c b/gdb/psymtab.c > index ac0ee0a5a6..e9a6a23b9d 100644 > --- a/gdb/psymtab.c > +++ b/gdb/psymtab.c > @@ -1621,17 +1623,17 @@ psymbol_hash (const void *addr, int length) > { > unsigned long h = 0; > struct partial_symbol *psymbol = (struct partial_symbol *) addr; > - unsigned int lang = psymbol->ginfo.language; > + unsigned int lang = PSYMBOL_LANGUAGE (psymbol); > unsigned int domain = PSYMBOL_DOMAIN (psymbol); > unsigned int theclass = PSYMBOL_CLASS (psymbol); > > - h = hash_continue (&psymbol->ginfo.value, sizeof (psymbol->ginfo.value), h); > + h = hash_continue (&psymbol->pginfo.value, sizeof (psymbol->pginfo.value), h); > h = hash_continue (&lang, sizeof (unsigned int), h); > h = hash_continue (&domain, sizeof (unsigned int), h); > h = hash_continue (&theclass, sizeof (unsigned int), h); > /* Note that psymbol names are interned via symbol_set_names, so > there's no need to hash the contents of the name here. */ > - h = hash_continue (&psymbol->ginfo.name, sizeof (psymbol->ginfo.name), h); > + h = hash_continue (&psymbol->pginfo.name, sizeof (psymbol->pginfo.name), h); I realize this is just a renaming of the existing code, but since there is an accessor for `name', I think it better to use it (in case someone ends up grepping for this). > > return h; > } > @@ -1646,15 +1648,15 @@ psymbol_compare (const void *addr1, const void *addr2, int length) > struct partial_symbol *sym1 = (struct partial_symbol *) addr1; > struct partial_symbol *sym2 = (struct partial_symbol *) addr2; > > - return (memcmp (&sym1->ginfo.value, &sym2->ginfo.value, > - sizeof (sym1->ginfo.value)) == 0 > - && sym1->ginfo.language == sym2->ginfo.language > + return (memcmp (&sym1->pginfo.value, &sym2->pginfo.value, > + sizeof (sym1->pginfo.value)) == 0 > + && sym1->pginfo.language == sym2->pginfo.language > && PSYMBOL_DOMAIN (sym1) == PSYMBOL_DOMAIN (sym2) > && PSYMBOL_CLASS (sym1) == PSYMBOL_CLASS (sym2) > /* Note that psymbol names are interned via > symbol_set_names, so there's no need to compare the > contents of the name here. */ > - && sym1->ginfo.name == sym2->ginfo.name); > + && sym1->pginfo.name == sym2->pginfo.name); > } Same here for PSYMBOL_LANGUAGE and PSYMBOL_LINKAGE_NAME? > /* Initialize a partial symbol bcache. */ Keith