From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from relay11.mail.gandi.net (relay11.mail.gandi.net [217.70.178.231]) by sourceware.org (Postfix) with ESMTPS id 7DEA339960E1 for ; Fri, 12 Mar 2021 15:04:58 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 7DEA339960E1 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 relay11.mail.gandi.net (Postfix) with ESMTPSA id 377BC100013; Fri, 12 Mar 2021 15:04:55 +0000 (UTC) Received: by localhost (Postfix, from userid 1000) id 8638658000E; Fri, 12 Mar 2021 16:04:52 +0100 (CET) From: Dodji Seketeli To: Matthias Maennich Cc: libabigail@sourceware.org, gprocida@google.com, kernel-team@android.com Subject: Re: [PATCH 06/20] Integrate new symtab reader into corpus and read_context Organization: Me, myself and I References: <20200619214305.562-1-maennich@google.com> <20210127125853.886677-1-maennich@google.com> <20210127125853.886677-7-maennich@google.com> X-Operating-System: Fedora 34 X-URL: http://www.seketeli.net/~dodji Date: Fri, 12 Mar 2021 16:04:52 +0100 In-Reply-To: <20210127125853.886677-7-maennich@google.com> (Matthias Maennich's message of "Wed, 27 Jan 2021 12:58:39 +0000") Message-ID: <87h7lgza3v.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, RCVD_IN_MSPIKE_H2, 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: Fri, 12 Mar 2021 15:05:01 -0000 Hello, Matthias Maennich a =C3=A9crit: > While reading the corpus in the read_context, also load the new type > symtab object side-by-side and set it accordingly in the resulting > corpus. This is still side by side and passive code that gets active in > the following changes. This is applicable for the dwarf reader as well > as for the reader that consumes XML. I see, this makes sense. [...] > diff --git a/include/abg-fwd.h b/include/abg-fwd.h > index bb839fe76315..76df1b51dae7 100644 > --- a/include/abg-fwd.h > +++ b/include/abg-fwd.h > @@ -1375,6 +1375,14 @@ typedef vector suppressions_type; >=20=20 > } // end namespace suppr >=20=20 > +namespace symtab_reader > +{ > + > +class symtab; > +typedef std::shared_ptr symtab_sptr; We'd need a small comment here so that this shows up well in the API doc. [...] > +++ b/src/abg-corpus.cc > @@ -23,6 +23,7 @@ ABG_BEGIN_EXPORT_DECLARATIONS > #include "abg-ir.h" > #include "abg-reader.h" > #include "abg-sptr-utils.h" > +#include "abg-symtab-reader.h" > #include "abg-tools-utils.h" > #include "abg-writer.h" >=20=20 > @@ -850,6 +851,14 @@ corpus::operator=3D=3D(const corpus& other) const > && j =3D=3D other.get_translation_units().end()); > } >=20=20 > +void > +corpus::set_symtab(symtab_reader::symtab_sptr symtab) > +{priv_->symtab_ =3D symtab;} > + > +const symtab_reader::symtab_sptr& > +corpus::get_symtab() const > +{ return priv_->symtab_; } > + Likewise for these two function definitions. [...] > --- a/src/abg-dwarf-reader.cc > +++ b/src/abg-dwarf-reader.cc [...] > + const symtab_reader::symtab_sptr& > + symtab() const > + { Likewise. > + if (!symtab_) > + symtab_ =3D symtab_reader::symtab::load( > + elf_handle(), options_.env, [&](const elf_symbol_sptr& symbol) { > + return is_elf_symbol_suppressed(symbol); > + }); Throughout the code, we block braces always go on the new line. So let's try to do the same for lambdas, e.g: symtab_ =3D symtab_reader::symtab::load (elf_handle(), options_.env, [&](const elf_symbol_sptr& symbol) { return is_elf_symbol_suppressed(symbol); }); I guess I should update the coding standard guidelines with the case for lambdas, etc, at some point. [...] > diff --git a/tests/test-symtab.cc b/tests/test-symtab.cc > index ffb20a4dec44..ff467d8ad345 100644 > --- a/tests/test-symtab.cc > +++ b/tests/test-symtab.cc [...] > - REQUIRE(status > - =3D=3D (dwarf_reader::STATUS_OK > - | dwarf_reader::STATUS_DEBUG_INFO_NOT_FOUND)); > + REQUIRE( > + status > + =3D=3D (dwarf_reader::STATUS_OK | dwarf_reader::STATUS_DEBUG_INFO_NO= T_FOUND)); I think this change is useless. > } >=20=20 [...] > > * include/abg-corpus.h (corpus::set_symtab): New method declaration. > (corpus::get_symtab): New method declaration. > * include/abg-fwd.h (symtab_reader::symtab_sptr): New forward > declaration. > * src/abg-corpus-priv.h (corpus::priv::symtab_): New data member. > * src/abg-corpus.cc > (corpus::set_symtab): Likewise. > (corpus::get_symtab): Likewise. > * src/abg-dwarf-reader.cc (read_context::symtab_): New data member. > (read_context::initialize): reset symtab_ as well > (read_context::symtab): new method that loads a symtab on > first access and returns it. > (read_debug_info_into_corpus): also set the new symtab object > on the current corpus. > (read_corpus_from_elf): Also determine (i.e. load) the new > symtab object and contribute to the load status. > * src/abg-reader.cc (read_corpus_from_input): also set the new > type symtab when reading from xml. > * tests/test-symtab.cc: Add test assertions. > > Reviewed-by: Giuliano Procida > Signed-off-by: Matthias Maennich This is OK to go in with the changes stated above and once the previous patch is applied. Thanks! Cheers, --=20 Dodji