From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from relay13.mail.gandi.net (relay13.mail.gandi.net [217.70.178.233]) by sourceware.org (Postfix) with ESMTPS id 05B16385480F for ; Tue, 16 Mar 2021 18:08:14 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 05B16385480F Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=seketeli.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=dodji@seketeli.org Received: from localhost (unknown [88.120.130.27]) (Authenticated sender: dodji@seketeli.org) by relay13.mail.gandi.net (Postfix) with ESMTPSA id 7485380003; Tue, 16 Mar 2021 18:08:13 +0000 (UTC) Received: by localhost (Postfix, from userid 1000) id CF19C58000E; Tue, 16 Mar 2021 19:08:12 +0100 (CET) From: Dodji Seketeli To: Matthias Maennich Cc: libabigail@sourceware.org, gprocida@google.com, kernel-team@android.com Subject: Re: [PATCH 14/20] abg-corpus: remove symbol maps and their setters Organization: Me, myself and I References: <20200619214305.562-1-maennich@google.com> <20210127125853.886677-1-maennich@google.com> <20210127125853.886677-15-maennich@google.com> X-Operating-System: Fedora 34 X-URL: http://www.seketeli.net/~dodji Date: Tue, 16 Mar 2021 19:08:12 +0100 In-Reply-To: <20210127125853.886677-15-maennich@google.com> (Matthias Maennich's message of "Wed, 27 Jan 2021 12:58:47 +0000") Message-ID: <87ft0vx983.fsf@seketeli.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-10.0 required=5.0 tests=BAYES_00, GIT_PATCH_0, JMQ_SPF_NEUTRAL, KAM_DMARC_STATUS, RCVD_IN_DNSWL_LOW, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: libabigail@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Mailing list of the Libabigail project List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 16 Mar 2021 18:08:17 -0000 Hello, Matthias Maennich a =C3=A9crit: [...] > diff --git a/src/abg-corpus.cc b/src/abg-corpus.cc > index f9f51a360707..94615bc727ff 100644 > --- a/src/abg-corpus.cc > +++ b/src/abg-corpus.cc > @@ -331,6 +331,18 @@ corpus::priv::get_sorted_fun_symbols() const > return *sorted_fun_symbols; > } >=20=20 > +const string_elf_symbols_map_type& > +corpus::priv::get_fun_symbol_map() const Please add a comment for this function. > +{ > + if (!fun_symbol_map) > + { > + fun_symbol_map =3D string_elf_symbols_map_type(); > + for (const auto& symbol : get_sorted_fun_symbols()) > + (*fun_symbol_map)[symbol->get_name()].push_back(symbol); > + } > + return *fun_symbol_map; > +} > + [...] > +const string_elf_symbols_map_type& > +corpus::priv::get_undefined_fun_symbol_map() const Likewise. > +{ > + if (!undefined_fun_symbol_map) > + { > + undefined_fun_symbol_map =3D string_elf_symbols_map_type(); > + for (const auto& symbol : get_sorted_undefined_fun_symbols()) > + (*undefined_fun_symbol_map)[symbol->get_name()].push_back(symbol); > + } > + return *undefined_fun_symbol_map; > +} > + > /// Return a list of symbols that are not referenced by any function of > /// corpus::get_functions(). > /// > @@ -425,6 +449,18 @@ corpus::priv::get_sorted_var_symbols() const > return *sorted_var_symbols; > } >=20=20 > +const string_elf_symbols_map_type& > +corpus::priv::get_var_symbol_map() const Likewise. > +{ > + if (!var_symbol_map) > + { > + var_symbol_map =3D string_elf_symbols_map_type(); > + for (const auto& symbol : get_sorted_var_symbols()) > + (*var_symbol_map)[symbol->get_name()].push_back(symbol); > + } > + return *var_symbol_map; > +} > + > /// Getter for a sorted vector of the variable symbols undefined in > /// this corpus. > /// > @@ -446,6 +482,18 @@ corpus::priv::get_sorted_undefined_var_symbols() con= st > return *sorted_undefined_var_symbols; > } >=20=20 > +const string_elf_symbols_map_type& > +corpus::priv::get_undefined_var_symbol_map() const Likewise. > +{ > + if (!undefined_var_symbol_map) > + { > + undefined_var_symbol_map =3D string_elf_symbols_map_type(); > + for (const auto& symbol : get_sorted_undefined_var_symbols()) > + (*undefined_var_symbol_map)[symbol->get_name()].push_back(symbol); > + } > + return *undefined_var_symbol_map; > +} > + [...] > With the prework in previous commits, we are now able to drop the > public symbols maps in corpus::priv and replace them by private members > with access through getters. The getters use the new symtab > implementation to generate the maps on the fly. Setters are not required > anymore and are removed. Also remove redundant getters. > > We could also remove the getters for the symbol maps and the local > caching variable and leave it all to lookup_symbol, but this is left for > a later change. > > * include/abg-corpus.h (corpus::set_fun_symbol_map): Remove > method declaration. > (corpus::set_undefined_fun_symbol_map): Likewise. > (corpus::set_var_symbol_map): Likewise. > (corpus::set_undefined_var_symbol_map): Likewise. > (corpus::get_fun_symbol_map_sptr): Likewise. > (corpus::get_undefined_fun_symbol_map_sptr): Likewise. > (corpus::get_var_symbol_map_sptr): Likewise. > (corpus::get_undefined_var_symbol_map_sptr): Likewise. > * src/abg-corpus-priv.h (corpus::priv::var_symbol_map): make > private and mutable > (corpus::priv::undefined_var_symbol_map): Likewise. > (corpus::priv::fun_symbol_map): Likewise. > (corpus::priv::undefined_fun_symbol_map): Likewise. > (corpus::priv::get_fun_symbol_map): New method declaration. > (corpus::priv::get_undefined_fun_symbol_map): Likewise. > (corpus::priv::get_var_symbol_map): Likewise. > (corpus::priv::get_undefined_var_symbol_map): Likewise. > * src/abg-corpus.cc (corpus::priv::get_fun_symbol_map): New > method implementation. > (corpus::priv::get_undefined_fun_symbol_map): Likewise. > (corpus::priv::get_var_symbol_map): Likewise. > (corpus::priv::get_undefined_var_symbol_map): Likewise. > (corpus::is_empty): depend on symtab only. > (corpus::set_fun_symbol_map): Remove method. > (corpus::set_undefined_fun_symbol_map): Likewise. > (corpus::set_var_symbol_map): Likewise. > (corpus::set_undefined_var_symbol_map): Likewise. > (corpus::get_fun_symbol_map_sptr): Likewise. > (corpus::get_undefined_fun_symbol_map_sptr): Likewise. > (corpus::get_var_symbol_map_sptr): Likewise. > (corpus::get_undefined_var_symbol_map_sptr): Likewise. > (corpus::get_fun_symbol_map): Use corpus::priv proxy method. > (corpus::get_undefined_fun_symbol_map): Likewise. > (corpus::get_var_symbol_map): Likewise. > (corpus::get_undefined_var_symbol_map): Likewise. > * src/abg-dwarf-reader.cc (read_debug_info_into_corpus): Do not > set corpus symbol maps anymore. > * src/abg-reader.cc (read_corpus_from_input): Likewise. > * tests/test-symtab.cc (assert_symbol_count): Do not access the > corpus symbol maps through sptr anymore. > * tests/data/test-read-dwarf/PR25007-sdhci.ko.abi: Adjust > expected test output. > > Reviewed-by: Giuliano Procida > Signed-off-by: Matthias Maennich OK to apply to master with the changes above once the prerequisite patches have been applied. Thanks! [...] Cheers, --=20 Dodji