From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from relay2-d.mail.gandi.net (relay2-d.mail.gandi.net [217.70.183.194]) by sourceware.org (Postfix) with ESMTPS id 95C563858D29 for ; Mon, 15 Mar 2021 10:05:27 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 95C563858D29 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 X-Originating-IP: 88.120.130.27 Received: from localhost (unknown [88.120.130.27]) (Authenticated sender: dodji@seketeli.org) by relay2-d.mail.gandi.net (Postfix) with ESMTPSA id 2CB2F40003; Mon, 15 Mar 2021 10:05:24 +0000 (UTC) Received: by localhost (Postfix, from userid 1000) id 7BF7158000E; Mon, 15 Mar 2021 11:05:24 +0100 (CET) From: Dodji Seketeli To: Matthias Maennich Cc: libabigail@sourceware.org, gprocida@google.com, kernel-team@android.com Subject: Re: [PATCH 07/20] corpus: make get_(undefined_)?_(var|fun)_symbols use the new symtab Organization: Me, myself and I References: <20200619214305.562-1-maennich@google.com> <20210127125853.886677-1-maennich@google.com> <20210127125853.886677-8-maennich@google.com> X-Operating-System: Fedora 34 X-URL: http://www.seketeli.net/~dodji Date: Mon, 15 Mar 2021 11:05:24 +0100 In-Reply-To: <20210127125853.886677-8-maennich@google.com> (Matthias Maennich's message of "Wed, 27 Jan 2021 12:58:40 +0000") Message-ID: <87czw0zq8r.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.1 required=5.0 tests=BAYES_00, GIT_PATCH_0, JMQ_SPF_NEUTRAL, KAM_DMARC_STATUS, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, 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: Mon, 15 Mar 2021 10:05:30 -0000 Hello, Matthias Maennich a =C3=A9crit: [...] > diff --git a/src/abg-corpus.cc b/src/abg-corpus.cc > index 0d5849ddac5d..75f897eb72a8 100644 > --- a/src/abg-corpus.cc > +++ b/src/abg-corpus.cc [...] > -/// Return a sorted vector of function symbols for this corpus. > -/// > -/// Note that the first time this function is called, the symbols are > -/// sorted and cached. Subsequent invocations of this function return > -/// the cached vector that was built previously. > -/// > -/// @return the sorted list of function symbols. Please keep this function comment even if it's redundant. We need this for the API doc to be complete. > const elf_symbols& > corpus::get_sorted_fun_symbols() const > -{ > - if (priv_->sorted_fun_symbols.empty() > - && !get_fun_symbol_map().empty()) > - { > - priv_->sorted_fun_symbols.reserve(get_fun_symbol_map().size()); > - for (string_elf_symbols_map_type::const_iterator i =3D > - get_fun_symbol_map().begin(); > - i !=3D get_fun_symbol_map().end(); > - ++i) > - for (elf_symbols::const_iterator s =3D i->second.begin(); > - s !=3D i->second.end(); > - ++s) > - priv_->sorted_fun_symbols.push_back(*s); > +{ return priv_->get_sorted_fun_symbols(); } Please remove the leading and trailing white space. [...] > -/// Getter for a sorted vector of the function symbols undefined in > -/// this corpus. > -/// > -/// @return a vector of the function symbols undefined in this corpus, > -/// sorted by name and then version. Likewise. > const elf_symbols& > corpus::get_sorted_undefined_fun_symbols() const > -{ > - if (priv_->sorted_undefined_fun_symbols.empty() > - && !get_undefined_fun_symbol_map().empty()) > - { > - priv_->sorted_undefined_fun_symbols.reserve > - (get_undefined_fun_symbol_map().size()); > - for (string_elf_symbols_map_type::const_iterator i =3D > - get_undefined_fun_symbol_map().begin(); > - i !=3D get_undefined_fun_symbol_map().end(); > - ++i) > - for (elf_symbols::const_iterator s =3D i->second.begin(); > - s !=3D i->second.end(); > - ++s) > - priv_->sorted_undefined_fun_symbols.push_back(*s); > +{ return priv_->get_sorted_undefined_fun_symbols(); } Likewise. [...] > +const elf_symbols& > +corpus::get_sorted_var_symbols() const Likewise. > +{ return priv_->get_sorted_var_symbols(); } Likewise. > +const elf_symbols& > +corpus::get_sorted_undefined_var_symbols() const Likewise. > +{ return priv_->get_sorted_undefined_var_symbols(); } Likewise. [...] > Make the corresponding members an implementation detail of corpus::priv. > They get computed based on the new symtab whenever they are accessed > first with an atomic instantiation. That simplifies the implementation > and homogenizes the access to functions and variables. Sorting does not > need to be done as the symtab already gives a guarantee for that. > > Due to improved alias detection in the new symtab reader, ensure we only > write symbol aliases to ksymtab symbols if having a ksymtab main symbol. > > Test data needed to be adjusted as the new symtab reader is stricter in > regards to symbols listed in ksymtab. I.e. init_module is not an > exported symbol in the ksymtab of a kernel module. > > * src/abg-corpus-priv.h (corpus::priv::sorted_var_symbols): make > private, mutable and optional. > (corpus::sorted_undefined_var_symbols): Likewise. > (corpus::sorted_fun_symbols): Likewise. > (corpus::sorted_undefined_fun_symbols): Likewise. > (corpus::priv::get_sorted_fun_symbols): New method declaration. > (corpus::priv::get_sorted_undefined_fun_symbols): Likewise. > (corpus::priv::get_sorted_var_symbols): Likewise. > (corpus::priv::get_sorted_undefined_var_symbols): Likewise. > * src/abg-corpus.cc > (corpus::elf_symbol_comp_functor): Delete struct. > (corpus::priv::get_sorted_fun_symbols): New method implementation. > (corpus::priv::get_sorted_undefined_fun_symbols): Likewise. > (corpus::priv::get_sorted_var_symbols): Likewise. > (corpus::priv::get_sorted_undefined_var_symbols): Likewise. > (corpus::get_sorted_fun_symbols): Proxy call to corpus::priv. > (corpus::get_sorted_undefined_fun_symbols): Likewise. > (corpus::get_sorted_var_symbols): Likewise. > (corpus::get_sorted_undefined_var_symbols): Likewise. > * src/abg-writer.cc (write_elf_symbol_aliases): When emitting > aliases for a kernel symbol, ensure to only emit exported, > public aliases. > * tests/data/test-read-dwarf/PR25007-sdhci.ko.abi: update test > data. > > Reviewed-by: Giuliano Procida > Signed-off-by: Matthias Maennich OK to apply to master with the above changes once the previous patches have been applied. Thanks! [...] Cheers, --=20 Dodji