From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from relay9-d.mail.gandi.net (relay9-d.mail.gandi.net [217.70.183.199]) by sourceware.org (Postfix) with ESMTPS id A86B13858034 for ; Sun, 21 Mar 2021 11:59:31 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org A86B13858034 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 relay9-d.mail.gandi.net (Postfix) with ESMTPSA id B32E2FF807; Sun, 21 Mar 2021 11:59:29 +0000 (UTC) Received: by localhost (Postfix, from userid 1000) id A96D7581C5C; Fri, 19 Mar 2021 19:15:59 +0100 (CET) From: Dodji Seketeli To: Matthias Maennich Cc: libabigail@sourceware.org, gprocida@google.com, kernel-team@android.com Subject: Re: [PATCH 20/20] symtab: Add support for MODVERSIONS (CRC checksums) Organization: Me, myself and I References: <20200619214305.562-1-maennich@google.com> <20210127125853.886677-1-maennich@google.com> <20210127125853.886677-21-maennich@google.com> X-Operating-System: Fedora 34 X-URL: http://www.seketeli.net/~dodji Date: Fri, 19 Mar 2021 19:15:59 +0100 In-Reply-To: <20210127125853.886677-21-maennich@google.com> (Matthias Maennich's message of "Wed, 27 Jan 2021 12:58:53 +0000") Message-ID: <871rcbvwkg.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=-9.0 required=5.0 tests=BAYES_00, DATE_IN_PAST_24_48, 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: Sun, 21 Mar 2021 11:59:33 -0000 Matthias Maennich a =C3=A9crit: [...] > diff --git a/src/abg-ir.cc b/src/abg-ir.cc > index 0b31f2014189..64dfffbf4011 100644 > --- a/src/abg-ir.cc > +++ b/src/abg-ir.cc [...] > @@ -1414,6 +1419,7 @@ elf_symbol::elf_symbol(const environment* e, > visibility vi, > bool is_linux_string_cst, > bool is_in_ksymtab, > + uint64_t crc, Please add a comment for this new parameter. > bool is_suppressed) > : priv_(new priv(e, > i, > @@ -1427,6 +1433,7 @@ elf_symbol::elf_symbol(const environment* e, > vi, > is_linux_string_cst, > is_in_ksymtab, > + crc, Likewise. > is_suppressed)) > {} >=20=20 > @@ -1486,11 +1493,12 @@ elf_symbol::create(const environment* e, > visibility vi, > bool is_linux_string_cst, > bool is_in_ksymtab, > + uint64_t crc, Likewise. [...] > +uint64_t > +elf_symbol::get_crc() const > +{return priv_->crc_;} > + > +void > +elf_symbol::set_crc(uint64_t crc) > +{priv_->crc_ =3D crc;} > + Please add a comment for these two accessors. [...] > diff --git a/src/abg-symtab-reader.cc b/src/abg-symtab-reader.cc > index 512fbc20adc0..8ee90c590a94 100644 > --- a/src/abg-symtab-reader.cc > +++ b/src/abg-symtab-reader.cc > @@ -12,6 +12,7 @@ [...] > @@ -338,6 +345,18 @@ symtab::load_(Elf* elf_handle, > has_ksymtab_entries_ =3D true; > } >=20=20 > + // Now add the CRC values > + for (const auto& crc_entry : crc_values) > + { > + const auto r =3D name_symbol_map_.find(crc_entry.first); > + if (r =3D=3D name_symbol_map_.end()) > + continue; > + > + for (const auto& symbol : r->second) { The opening parenthesis is useless (and should have go on the next line if it was not) > + symbol->set_crc(crc_entry.second); > + } > + } > + > // sort the symbols for deterministic output > std::sort(symbols_.begin(), symbols_.end(), symbol_sort); >=20=20 [...] > > * include/abg-ir.h (elf_symbol::elf_symbol): Add CRC parameter. > (elf_symbol::create): Likewise. > (elf_symbol::get_crc): New member method. > (elf_symbol::set_crc): New member method. > * src/abg-comp-filter.cc (crc_changed): New function. > (categorize_harmful_diff_node): Also test for CRC changes. > * src/abg-ir.cc (elf_symbol::priv::crc_): New data member. > * src/abg-ir.cc (elf_symbol::priv::priv): Add CRC parameter. > (elf_symbol::elf_symbol): Likewise. > (elf_symbol::create): Likewise. > (elf_symbol::textually_equals): Add CRC support. > (elf_symbol::get_crc): New member method. > (elf_symbol::set_crc): New member method. > * src/abg-reader.cc (build_elf_symbol): Add CRC support. > * src/abg-reporter-priv.cc (maybe_report_diff_for_symbol): Likewise. > * src/abg-symtab-reader.cc (symtab::load): Likewise. > * src/abg-writer.cc (write_elf_symbol): Likewise. > * tests/data/Makefile.am: Add new test data files. > * tests/data/test-abidiff-exit/test-crc-report.txt: New test file. > * tests/data/test-abidiff-exit/test-crc-v0.abi: Likewise. > * tests/data/test-abidiff-exit/test-crc-v1.abi: Likewise. > * tests/data/test-abidiff/empty-report.txt: New file. > * tests/data/test-abidiff/test-PR18166-libtirpc.so.report.txt: Deleted. > * tests/data/test-abidiff/test-PR24552-report0.txt: Deleted. > * tests/data/test-abidiff/test-crc-0.xml: New test file. > * tests/data/test-abidiff/test-crc-1.xml: Likewise. > * tests/data/test-abidiff/test-crc-2.xml: Likewise. > * tests/data/test-abidiff/test-crc-report.txt: Likewise. > * tests/data/test-abidiff/test-empty-corpus-report.txt: Deleted. > * tests/data/test-read-dwarf/PR25007-sdhci.ko.abi: Add CRC values. > * tests/data/test-read-write/test-crc.xml: New test data file. > * tests/data/test-symtab/kernel-modversions/Makefile: New test source. > * tests/data/test-symtab/kernel-modversions/one_of_each.c: Likewise. > * tests/data/test-symtab/kernel-modversions/one_of_each.ko: Likewise. > * tests/test-abidiff-exit.cc: Add new test case. > * tests/test-abidiff.cc: Add new test case. > * tests/test-read-write.cc: Likewise. > * tests/test-symtab.cc (Symtab::KernelSymtabsWithCRC): New test case. > > Reviewed-by: Giuliano Procida > Signed-off-by: Matthias Maennich OK to apply to master once the pre-requisites are in. Thanks! [...] Cheers, --=20 Dodji