From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr1-x442.google.com (mail-wr1-x442.google.com [IPv6:2a00:1450:4864:20::442]) by sourceware.org (Postfix) with ESMTPS id D412438708D0 for ; Mon, 27 Apr 2020 11:01:51 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org D412438708D0 Received: by mail-wr1-x442.google.com with SMTP id d15so18388793wrx.3 for ; Mon, 27 Apr 2020 04:01:51 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=aFBOcShTqqSSxBazXAs7Oy/6iMbFY5PWOteRtzUPzoE=; b=SXRFAcetTK5KTel7yBbfZ1PJa8PjRoIfcibVP9sfP73ZrscAin4QauqK54UNdDGUPe A2uQCZdidrcntsJSDhQ5QRJxuCb3NB78uP2CHFENRJqUXgQ8+i/SRJz9IdC01qB3SqNe wiQYOaqjGkjPU1+D2+X4oQ5nfb4YK7AKGjLDMvsKZQ32d0DWIxSXOdm3qQYuMo4DCgC0 MJ41A2qKvxwIkO/tCHYqZYcg3m7P0ZqNKlmAfO0K43CzFzys3pNRQCq9bltgSNjPbkoz wHT0M5UypxP9jQ9Zn8cWB4t9eO3zpgfOEq9XtXLGwH7lTTeAL7Nkwvw8yMMawGvNf93U CKzQ== X-Gm-Message-State: AGi0PubkMLQQ0t0QnL7U/e5PKLXJOMCqpWn2r2r4vRU9809i/y+D1geJ U3sVyDIPapk4BtH8q5271Tccuw== X-Google-Smtp-Source: APiQypKwd0du6cXqGyqONFFSM0a+Ey2PHMaizZv6UHgxPskUt3Wdc8TxITBNdf8Ko7Zd4KrVMBf8BQ== X-Received: by 2002:adf:9342:: with SMTP id 60mr25855613wro.129.1587985310554; Mon, 27 Apr 2020 04:01:50 -0700 (PDT) Received: from google.com ([2a00:79e0:d:210:e8f7:125b:61e9:733d]) by smtp.gmail.com with ESMTPSA id n6sm22319048wrs.81.2020.04.27.04.01.49 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 27 Apr 2020 04:01:50 -0700 (PDT) Date: Mon, 27 Apr 2020 13:01:49 +0200 From: Matthias Maennich To: Giuliano Procida Cc: libabigail@sourceware.org, dodji@seketeli.org, kernel-team@android.com Subject: Re: [PATCH v3 03/21] Simplify generation of symbol whitelist regex. Message-ID: <20200427110149.GC159704@google.com> References: <20200423154441.170531-1-gprocida@google.com> <20200424092132.150547-1-gprocida@google.com> <20200424092132.150547-4-gprocida@google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii; format=flowed Content-Disposition: inline In-Reply-To: <20200424092132.150547-4-gprocida@google.com> X-Spam-Status: No, score=-41.3 required=5.0 tests=BAYES_00, DKIMWL_WL_MED, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, ENV_AND_HDR_SPF_MATCH, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, USER_IN_DEF_DKIM_WL, USER_IN_DEF_SPF_WL 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: Libabigail mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 27 Apr 2020 11:01:55 -0000 On Fri, Apr 24, 2020 at 10:21:14AM +0100, Giuliano Procida wrote: >The code to build the symbol whitelist regex uses things like seekp >and tellp to generate regexes like "^foo$|^bar$". > >This patch simplifies the code, for further enhancement, resulting in >generated regexes like "^(foo|bar)$". > >There should be no change in behaviour, unless whitelisted symbol >names contain special regex characters. > > * include/abg-regex.h (generate_from_strings): Declare new > function to build a regex from some strings, representing a > membership test. > * src/abg-regex.cc (generate_from_strings): Implement new > function to build a regex from some strings, representing a > membership test, in a straightfoward fashion. > * src/abg-tools-utils.cc > (gen_suppr_spec_from_kernel_abi_whitelists): Replace > regex-building code with a call to generate_from_strings. > * tests/test-kmi-whitelist.cc: Update regexes in test. > >Signed-off-by: Giuliano Procida >--- > include/abg-regex.h | 6 ++++++ > src/abg-regex.cc | 28 ++++++++++++++++++++++++++++ > src/abg-tools-utils.cc | 10 +++------- > tests/test-kmi-whitelist.cc | 10 +++++----- > 4 files changed, 42 insertions(+), 12 deletions(-) > >diff --git a/include/abg-regex.h b/include/abg-regex.h >index 84c386a9..2f638ef2 100644 >--- a/include/abg-regex.h >+++ b/include/abg-regex.h >@@ -27,6 +27,9 @@ > > #include > >+#include >+#include >+ > #include "abg-cxx-compat.h" > #include "abg-sptr-utils.h" > >@@ -55,6 +58,9 @@ struct regex_t_deleter > } > };//end struct regex_deleter > >+std::string >+generate_from_strings(const std::vector& strs); >+ > }// end namespace regex > > /// Specialization of sptr_utils::build_sptr for regex_t. >diff --git a/src/abg-regex.cc b/src/abg-regex.cc >index 13f5841d..79a89033 100644 >--- a/src/abg-regex.cc >+++ b/src/abg-regex.cc >@@ -23,6 +23,7 @@ > /// Some specialization for shared pointer utility templates. > /// > >+#include > #include "abg-sptr-utils.h" > #include "abg-regex.h" > >@@ -52,4 +53,31 @@ regex::regex_t_sptr > sptr_utils::build_sptr() > {return sptr_utils::build_sptr(new regex_t);} > >+namespace regex >+{ >+ >+/// Generate a regex pattern equivalent to testing set membership. >+/// >+/// A string will match the resulting pattern regex, if and only if it >+/// was present in the vector. >+/// >+/// @param strs a vector of strings >+/// >+/// @return a regex pattern >+std::string >+generate_from_strings(const std::vector& strs) >+{ >+ if (strs.empty()) >+ return "^_^"; I am not aware of this constant. Looks like an invalid regex. Can you document it? Or am I missing the joke? :-) Cheers, Matthias >+ std::ostringstream os; >+ std::vector::const_iterator i = strs.begin(); >+ os << "^(" << *i++; >+ while (i != strs.end()) >+ os << "|" << *i++; >+ os << ")$"; >+ return os.str(); >+} >+ >+}//end namespace regex >+ > }//end namespace abigail >diff --git a/src/abg-tools-utils.cc b/src/abg-tools-utils.cc >index a06e8615..11486a21 100644 >--- a/src/abg-tools-utils.cc >+++ b/src/abg-tools-utils.cc >@@ -61,6 +61,8 @@ > #include "abg-dwarf-reader.h" > #include "abg-internal.h" > #include "abg-cxx-compat.h" >+#include "abg-regex.h" >+ > // > ABG_BEGIN_EXPORT_DECLARATIONS > >@@ -2002,13 +2004,7 @@ gen_suppr_spec_from_kernel_abi_whitelists > > // Build a regular expression representing the union of all > // the function and variable names expressed in the white list. >- std::stringstream regex_ss; >- regex_ss << "^"; >- std::copy(whitelisted_names.begin(), whitelisted_names.end(), >- std::ostream_iterator(regex_ss, "$|^")); >- regex_ss.seekp(0, std::ios::end); >- const std::string& regex = >- regex_ss.str().substr(0, static_cast(regex_ss.tellp()) - 2); >+ const std::string regex = regex::generate_from_strings(whitelisted_names); > > // Build a suppression specification which *keeps* functions > // whose ELF symbols match the regular expression contained >diff --git a/tests/test-kmi-whitelist.cc b/tests/test-kmi-whitelist.cc >index 2aa0f463..bcc5adee 100644 >--- a/tests/test-kmi-whitelist.cc >+++ b/tests/test-kmi-whitelist.cc >@@ -96,7 +96,7 @@ TEST_CASE("WhitelistWithASingleEntry", "[whitelists]") > suppressions_type suppr > = gen_suppr_spec_from_kernel_abi_whitelists(abi_whitelist_paths); > REQUIRE(!suppr.empty()); >- test_suppressions_are_consistent(suppr, "^test_symbol$"); >+ test_suppressions_are_consistent(suppr, "^(test_symbol)$"); > } > > TEST_CASE("WhitelistWithADuplicateEntry", "[whitelists]") >@@ -106,7 +106,7 @@ TEST_CASE("WhitelistWithADuplicateEntry", "[whitelists]") > suppressions_type suppr > = gen_suppr_spec_from_kernel_abi_whitelists(abi_whitelist_paths); > REQUIRE(!suppr.empty()); >- test_suppressions_are_consistent(suppr, "^test_symbol$"); >+ test_suppressions_are_consistent(suppr, "^(test_symbol)$"); > } > > TEST_CASE("TwoWhitelists", "[whitelists]") >@@ -118,7 +118,7 @@ TEST_CASE("TwoWhitelists", "[whitelists]") > gen_suppr_spec_from_kernel_abi_whitelists(abi_whitelist_paths); > REQUIRE(!suppr.empty()); > test_suppressions_are_consistent(suppr, >- "^test_another_symbol$|^test_symbol$"); >+ "^(test_another_symbol|test_symbol)$"); > } > > TEST_CASE("TwoWhitelistsWithDuplicates", "[whitelists]") >@@ -130,7 +130,7 @@ TEST_CASE("TwoWhitelistsWithDuplicates", "[whitelists]") > = gen_suppr_spec_from_kernel_abi_whitelists(abi_whitelist_paths); > REQUIRE(!suppr.empty()); > test_suppressions_are_consistent(suppr, >- "^test_another_symbol$|^test_symbol$"); >+ "^(test_another_symbol|test_symbol)$"); > } > > TEST_CASE("WhitelistWithTwoSections", "[whitelists]") >@@ -140,5 +140,5 @@ TEST_CASE("WhitelistWithTwoSections", "[whitelists]") > suppressions_type suppr > = gen_suppr_spec_from_kernel_abi_whitelists(abi_whitelist_paths); > REQUIRE(!suppr.empty()); >- test_suppressions_are_consistent(suppr, "^test_symbol1$|^test_symbol2$"); >+ test_suppressions_are_consistent(suppr, "^(test_symbol1|test_symbol2)$"); > } >-- >2.26.2.303.gf8c07b1a785-goog >