From patchwork Mon Aug 17 09:38:16 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Giuliano Procida X-Patchwork-Id: 40275 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id C40D1385700C; Mon, 17 Aug 2020 09:38:40 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org C40D1385700C DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1597657120; bh=sLpa2dmUch1LqhSyvGma/rQMUD83C2ElgWg0n9RkJ6Y=; h=Date:In-Reply-To:References:Subject:To:List-Id:List-Unsubscribe: List-Archive:List-Help:List-Subscribe:From:Reply-To:Cc:From; b=Ih8fodeaUL0lSAzDW5QQ7Ml4jRUt3pyz+beu9gY4yoGbgplEoxrtDEIVCQ3VWtCs4 l3IhxOQFKFX17k/wzWlPBM87gHlZUIgf0ktHA+lU3Izx1eNVegMMPzV7K9vBfl/riH jQCcvYF48oACZecw/Rc/QVjKO62K7TmXkZflnBSc= X-Original-To: libabigail@sourceware.org Delivered-To: libabigail@sourceware.org Received: from mail-qv1-xf4a.google.com (mail-qv1-xf4a.google.com [IPv6:2607:f8b0:4864:20::f4a]) by sourceware.org (Postfix) with ESMTPS id C8FC43858D37 for ; Mon, 17 Aug 2020 09:38:36 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org C8FC43858D37 Received: by mail-qv1-xf4a.google.com with SMTP id l18so10552501qvq.16 for ; Mon, 17 Aug 2020 02:38:36 -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:in-reply-to:message-id:mime-version :references:subject:from:to:cc; bh=sLpa2dmUch1LqhSyvGma/rQMUD83C2ElgWg0n9RkJ6Y=; b=YHoB4kZarl7PnGO3lA/Ut03IX8luEppxZx0JQ+czK/k3u8vauPzCBcHh2JkadgdXUn UrTyFi6phzRnCPSBuPDTfpAnRBNXbxbNz8kNINZU5/u+dEfWZ0l0HQUuDNylEemjxPLP PYoczJ6MvoxxcD8HpRQvNFf7fbvUutckUH90RJWN7c/OPnpEZ8oXUGxh2iMfhKfS217a H01WjbxYceDAkR7Kerjfbg6iifxRJJBivkPHH379IpEjw5kPcqV9aDXHbBB+dqO1Ih16 e1uvUNYYtE88KuYrwYVevfysgJ3OcJslIJYIeDXQbIXAxZ/ApO1MuqT32U/FM9i1NLnE 1wMQ== X-Gm-Message-State: AOAM5335kt2Ol4IUaOwGOd2AP6cV9BZ+Pg7erD+KISS9Zu7QO7z5/2iA jmlOusqqcXa+6H9xcrMTCUSIbbTLEabxX+Wlk35EfpL40Wnzf6D3nrHtunASlWfKPSoeHEz0sW9 UShnp0U39M+AEQIeljR/Yl5zTP+V/pvOWyU97YF/fi36yPVNZHJvrnzMQ5O6CYEozjV0H740= X-Google-Smtp-Source: ABdhPJzaRSUHK24g2atVly07UlAcL4AE0AglC7PiiYlYWrjywhPcddWxaWgMDrZE2n3wDBMnkBnuU6uC4U/36A== X-Received: by 2002:ad4:42cc:: with SMTP id f12mr13771599qvr.89.1597657116298; Mon, 17 Aug 2020 02:38:36 -0700 (PDT) Date: Mon, 17 Aug 2020 10:38:16 +0100 In-Reply-To: <20200817093819.172380-1-gprocida@google.com> Message-Id: <20200817093819.172380-5-gprocida@google.com> Mime-Version: 1.0 References: <20200817093819.172380-1-gprocida@google.com> X-Mailer: git-send-email 2.28.0.220.ged08abb693-goog Subject: [PATCH 4/7] Add read_*_suppression success/failure plumbing. To: libabigail@sourceware.org X-Spam-Status: No, score=-22.2 required=5.0 tests=BAYES_00, DKIMWL_WL_MED, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, USER_IN_DEF_DKIM_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: Mailing list of the Libabigail project List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-Patchwork-Original-From: Giuliano Procida via Libabigail From: Giuliano Procida Reply-To: Giuliano Procida Cc: kernel-team@android.com Errors-To: libabigail-bounces@sourceware.org Sender: "Libabigail" In order to track parse success/failure, we need all parsing functions to emit this as well any parsed values. The simplest way to do this is to return parse success and to assign parsed values to output parameters. This is a pattern that is already present in the code base. This patch does this for the four main parsing functions. * src/abg-suppression.cc (read_type_suppression): Return result via assignment to reference argument and return true iff input section was parsed successfully. (read_function_suppression): Ditto. (read_variable_suppression): Ditto. (read_file_suppression): Ditto. Signed-off-by: Giuliano Procida --- src/abg-suppression.cc | 250 +++++++++++++++++++++-------------------- 1 file changed, 129 insertions(+), 121 deletions(-) diff --git a/src/abg-suppression.cc b/src/abg-suppression.cc index 682bb8742..736c7f51f 100644 --- a/src/abg-suppression.cc +++ b/src/abg-suppression.cc @@ -346,17 +346,21 @@ names_of_binaries_match(const suppression_base& suppr, suppression_base::~suppression_base() {} -static type_suppression_sptr -read_type_suppression(const ini::config::section& section); +static bool +read_type_suppression(const ini::config::section& section, + suppression_sptr& suppr); -static function_suppression_sptr -read_function_suppression(const ini::config::section& section); +static bool +read_function_suppression(const ini::config::section& section, + suppression_sptr& suppr); -static variable_suppression_sptr -read_variable_suppression(const ini::config::section& section); +static bool +read_variable_suppression(const ini::config::section& section, + suppression_sptr& suppr); -static file_suppression_sptr -read_file_suppression(const ini::config::section& section); +static bool +read_file_suppression(const ini::config::section& section, + suppression_sptr& suppr); /// Read a vector of suppression specifications from the sections of /// an ini::config. @@ -381,13 +385,13 @@ read_suppressions(const ini::config& config, const std::string& name = section->get_name(); suppression_sptr s; if (name == "suppress_type") - s = read_type_suppression(*section); + read_type_suppression(*section, s); else if (name == "suppress_function") - s = read_function_suppression(*section); + read_function_suppression(*section, s); else if (name == "suppress_variable") - s = read_variable_suppression(*section); + read_variable_suppression(*section, s); else if (name == "suppress_file") - s = read_file_suppression(*section); + read_file_suppression(*section, s); if (s) suppressions.push_back(s); } @@ -1565,13 +1569,13 @@ read_suppression_reach_kind(const string& input) /// /// @param section the section of the ini config to read. /// -/// @return the resulting @ref type_suppression upon successful -/// parsing, or nil. -static type_suppression_sptr -read_type_suppression(const ini::config::section& section) +/// @param suppr the @ref suppression to assign to. +/// +/// @return whether the parse was successful. +static bool +read_type_suppression(const ini::config::section& section, + suppression_sptr& suppr) { - type_suppression_sptr result; - static const char *const sufficient_props[] = { "file_name_regexp", "file_name_not_regexp", @@ -1587,7 +1591,7 @@ read_type_suppression(const ini::config::section& section) if (!check_sufficient_props(sufficient_props, sizeof(sufficient_props)/sizeof(char*), section)) - return result; + return false; ini::simple_property_sptr drop_artifact = is_simple_property(section.find_property("drop_artifact")); @@ -1711,7 +1715,7 @@ read_type_suppression(const ini::config::section& section) type_suppression::insertion_range::create_fn_call_expr_boundary(ini::read_function_call_expr(ins_point))) begin = expr; else - return result; + return false; end = type_suppression::insertion_range::create_integer_boundary(-1); type_suppression::insertion_range_sptr insert_range @@ -1754,7 +1758,7 @@ read_type_suppression(const ini::config::section& section) type_suppression::insertion_range::create_fn_call_expr_boundary(ini::read_function_call_expr(str))) begin = expr; else - return result; + return false; str = val->get_content()[1]; if (str == "end") @@ -1767,7 +1771,7 @@ read_type_suppression(const ini::config::section& section) type_suppression::insertion_range::create_fn_call_expr_boundary(ini::read_function_call_expr(str))) end = expr; else - return result; + return false; type_suppression::insertion_range_sptr insert_range (new type_suppression::insertion_range(begin, end)); @@ -1778,7 +1782,7 @@ read_type_suppression(const ini::config::section& section) // the 'has_data_member_inserted_between' property has a wrong // value type, so let's discard the endire [suppress_type] // section. - return result; + return false; } // Support has_data_members_inserted_between @@ -1829,7 +1833,7 @@ read_type_suppression(const ini::config::section& section) type_suppression::insertion_range::create_fn_call_expr_boundary(ini::read_function_call_expr(str))) begin = expr; else - return result; + return false; str = list_value->get_content()[1]; if (str == "end") @@ -1842,7 +1846,7 @@ read_type_suppression(const ini::config::section& section) type_suppression::insertion_range::create_fn_call_expr_boundary(ini::read_function_call_expr(str))) end = expr; else - return result; + return false; type_suppression::insertion_range_sptr insert_range (new type_suppression::insertion_range(begin, end)); @@ -1850,7 +1854,7 @@ read_type_suppression(const ini::config::section& section) consider_data_member_insertion = true; } if (!is_well_formed) - return result; + return false; } /// Support 'changed_enumerators = foo, bar, baz' @@ -1877,56 +1881,57 @@ read_type_suppression(const ini::config::section& section) changed_enumerator_names.push_back(p->get_value()->as_string()); } - result.reset(new type_suppression(label_str, name_regex_str, name_str)); + type_suppression result(label_str, name_regex_str, name_str); if (consider_type_kind) { - result->set_consider_type_kind(true); - result->set_type_kind(type_kind); + result.set_consider_type_kind(true); + result.set_type_kind(type_kind); } if (consider_reach_kind) { - result->set_consider_reach_kind(true); - result->set_reach_kind(reach_kind); + result.set_consider_reach_kind(true); + result.set_reach_kind(reach_kind); } if (consider_data_member_insertion) - result->set_data_member_insertion_ranges(insert_ranges); + result.set_data_member_insertion_ranges(insert_ranges); if (!name_not_regex_str.empty()) - result->set_type_name_not_regex_str(name_not_regex_str); + result.set_type_name_not_regex_str(name_not_regex_str); if (!file_name_regex_str.empty()) - result->set_file_name_regex_str(file_name_regex_str); + result.set_file_name_regex_str(file_name_regex_str); if (!file_name_not_regex_str.empty()) - result->set_file_name_not_regex_str(file_name_not_regex_str); + result.set_file_name_not_regex_str(file_name_not_regex_str); if (!soname_regex_str.empty()) - result->set_soname_regex_str(soname_regex_str); + result.set_soname_regex_str(soname_regex_str); if (!soname_not_regex_str.empty()) - result->set_soname_not_regex_str(soname_not_regex_str); + result.set_soname_not_regex_str(soname_not_regex_str); if (!srcloc_not_in.empty()) - result->set_source_locations_to_keep(srcloc_not_in); + result.set_source_locations_to_keep(srcloc_not_in); if (!srcloc_not_regexp_str.empty()) - result->set_source_location_to_keep_regex_str(srcloc_not_regexp_str); + result.set_source_location_to_keep_regex_str(srcloc_not_regexp_str); if ((drop_artifact_str == "yes" || drop_artifact_str == "true") && ((!name_regex_str.empty() || !name_str.empty() || !srcloc_not_regexp_str.empty() || !srcloc_not_in.empty()))) - result->set_drops_artifact_from_ir(true); + result.set_drops_artifact_from_ir(true); - if (result->get_type_kind() == type_suppression::ENUM_TYPE_KIND + if (result.get_type_kind() == type_suppression::ENUM_TYPE_KIND && !changed_enumerator_names.empty()) - result->set_changed_enumerator_names(changed_enumerator_names); + result.set_changed_enumerator_names(changed_enumerator_names); - return result; + suppr.reset(new type_suppression(result)); + return true; } // @@ -3146,18 +3151,19 @@ read_parameter_spec_from_string(const string& str) return result; } -/// Parse function suppression specification, build a resulting @ref -/// function_suppression type and return a shared pointer to that -/// object. +/// Read a function suppression from an instance of +/// ini::config::section and build a @ref function_suppression as a +/// result. +/// +/// @param section the section of the ini config to read. /// -/// @return a shared pointer to the newly built @ref -/// function_suppression. If the function suppression specification -/// could not be parsed then a nil shared pointer is returned. -static function_suppression_sptr -read_function_suppression(const ini::config::section& section) +/// @param suppr the @ref suppression to assign to. +/// +/// @return whether the parse was successful. +static bool +read_function_suppression(const ini::config::section& section, + suppression_sptr& suppr) { - function_suppression_sptr result; - static const char *const sufficient_props[] = { "label", "file_name_regexp", @@ -3179,7 +3185,7 @@ read_function_suppression(const ini::config::section& section) if (!check_sufficient_props(sufficient_props, sizeof(sufficient_props)/sizeof(char*), section)) - return result; + return false; ini::simple_property_sptr drop_artifact = is_simple_property(section.find_property("drop_artifact")); @@ -3307,16 +3313,16 @@ read_function_suppression(const ini::config::section& section) parms.push_back(parm); } - result.reset(new function_suppression(label_str, - name, - name_regex_str, - return_type_name, - return_type_regex_str, - parms, - sym_name, - sym_name_regex_str, - sym_version, - sym_ver_regex_str)); + function_suppression result(label_str, + name, + name_regex_str, + return_type_name, + return_type_regex_str, + parms, + sym_name, + sym_name_regex_str, + sym_version, + sym_ver_regex_str); if ((drop_artifact_str == "yes" || drop_artifact_str == "true") && (!name.empty() @@ -3325,35 +3331,36 @@ read_function_suppression(const ini::config::section& section) || !sym_name.empty() || !sym_name_regex_str.empty() || !sym_name_not_regex_str.empty())) - result->set_drops_artifact_from_ir(true); + result.set_drops_artifact_from_ir(true); if (!change_kind_str.empty()) - result->set_change_kind + result.set_change_kind (function_suppression::parse_change_kind(change_kind_str)); if (!allow_other_aliases.empty()) - result->set_allow_other_aliases(allow_other_aliases == "yes" - || allow_other_aliases == "true"); + result.set_allow_other_aliases(allow_other_aliases == "yes" + || allow_other_aliases == "true"); if (!name_not_regex_str.empty()) - result->set_name_not_regex_str(name_not_regex_str); + result.set_name_not_regex_str(name_not_regex_str); if (!sym_name_not_regex_str.empty()) - result->set_symbol_name_not_regex_str(sym_name_not_regex_str); + result.set_symbol_name_not_regex_str(sym_name_not_regex_str); if (!file_name_regex_str.empty()) - result->set_file_name_regex_str(file_name_regex_str); + result.set_file_name_regex_str(file_name_regex_str); if (!file_name_not_regex_str.empty()) - result->set_file_name_not_regex_str(file_name_not_regex_str); + result.set_file_name_not_regex_str(file_name_not_regex_str); if (!soname_regex_str.empty()) - result->set_soname_regex_str(soname_regex_str); + result.set_soname_regex_str(soname_regex_str); if (!soname_not_regex_str.empty()) - result->set_soname_not_regex_str(soname_not_regex_str); + result.set_soname_not_regex_str(soname_not_regex_str); - return result; + suppr.reset(new function_suppression(result)); + return true; } // @@ -4023,18 +4030,19 @@ operator|(variable_suppression::change_kind l, (static_cast(l) | static_cast(r)); } -/// Parse variable suppression specification, build a resulting @ref -/// variable_suppression type and return a shared pointer to that -/// object. +/// Read a variable suppression from an instance of +/// ini::config::section and build a @ref variable_suppression as a +/// result. +/// +/// @param section the section of the ini config to read. +/// +/// @param suppr the @ref suppression to assign to. /// -/// @return a shared pointer to the newly built @ref -/// variable_suppression. If the variable suppression specification -/// could not be parsed then a nil shared pointer is returned. -static variable_suppression_sptr -read_variable_suppression(const ini::config::section& section) +/// @return whether the parse was successful. +static bool +read_variable_suppression(const ini::config::section& section, + suppression_sptr& suppr) { - variable_suppression_sptr result; - static const char *const sufficient_props[] = { "label", "file_name_regexp", @@ -4055,7 +4063,7 @@ read_variable_suppression(const ini::config::section& section) if (!check_sufficient_props(sufficient_props, sizeof(sufficient_props)/sizeof(char*), section)) - return result; + return false; ini::simple_property_sptr drop_artifact = is_simple_property(section.find_property("drop_artifact")); @@ -4162,15 +4170,15 @@ read_variable_suppression(const ini::config::section& section) ? type_name_regex_prop->get_value()->as_string() : ""; - result.reset(new variable_suppression(label_str, - name_str, - name_regex_str, - symbol_name, - symbol_name_regex_str, - symbol_version, - symbol_version_regex_str, - type_name_str, - type_name_regex_str)); + variable_suppression result(label_str, + name_str, + name_regex_str, + symbol_name, + symbol_name_regex_str, + symbol_version, + symbol_version_regex_str, + type_name_str, + type_name_regex_str); if ((drop_artifact_str == "yes" || drop_artifact_str == "true") && (!name_str.empty() @@ -4179,31 +4187,32 @@ read_variable_suppression(const ini::config::section& section) || !symbol_name.empty() || !symbol_name_regex_str.empty() || !symbol_name_not_regex_str.empty())) - result->set_drops_artifact_from_ir(true); + result.set_drops_artifact_from_ir(true); if (!name_not_regex_str.empty()) - result->set_name_not_regex_str(name_not_regex_str); + result.set_name_not_regex_str(name_not_regex_str); if (!symbol_name_not_regex_str.empty()) - result->set_symbol_name_not_regex_str(symbol_name_not_regex_str); + result.set_symbol_name_not_regex_str(symbol_name_not_regex_str); if (!change_kind_str.empty()) - result->set_change_kind + result.set_change_kind (variable_suppression::parse_change_kind(change_kind_str)); if (!file_name_regex_str.empty()) - result->set_file_name_regex_str(file_name_regex_str); + result.set_file_name_regex_str(file_name_regex_str); if (!file_name_not_regex_str.empty()) - result->set_file_name_not_regex_str(file_name_not_regex_str); + result.set_file_name_not_regex_str(file_name_not_regex_str); if (!soname_regex_str.empty()) - result->set_soname_regex_str(soname_regex_str); + result.set_soname_regex_str(soname_regex_str); if (!soname_not_regex_str.empty()) - result->set_soname_not_regex_str(soname_not_regex_str); + result.set_soname_not_regex_str(soname_not_regex_str); - return result; + suppr.reset(new variable_suppression(result)); + return true; } // @@ -4286,17 +4295,17 @@ file_suppression::~file_suppression() } /// Read a file suppression from an instance of ini::config::section -/// and build a @ref type_suppression as a result. +/// and build a @ref file_suppression as a result. +/// +/// @param section the section of the ini config to read. /// -/// @param section the section (from an ini file) to read the file -/// suppression from. +/// @param suppr the @ref suppression to assign to. /// -/// @return file_suppression_sptr. -static file_suppression_sptr -read_file_suppression(const ini::config::section& section) +/// @return whether the parse was successful. +static bool +read_file_suppression(const ini::config::section& section, + suppression_sptr& suppr) { - file_suppression_sptr result; - static const char *const sufficient_props[] = { "file_name_regexp", "file_name_not_regexp", @@ -4306,7 +4315,7 @@ read_file_suppression(const ini::config::section& section) if (!check_sufficient_props(sufficient_props, sizeof(sufficient_props)/sizeof(char*), section)) - return result; + return false; ini::simple_property_sptr label_prop = is_simple_property(section.find_property("label")); @@ -4338,23 +4347,22 @@ read_file_suppression(const ini::config::section& section) ? soname_not_regex_prop->get_value()->as_string() : ""; - result.reset(new file_suppression(label_str, - file_name_regex_str, - file_name_not_regex_str)); + file_suppression result(label_str, file_name_regex_str, file_name_not_regex_str); if (!soname_regex_str.empty()) { - result->set_soname_regex_str(soname_regex_str); - result->set_drops_artifact_from_ir(true); + result.set_soname_regex_str(soname_regex_str); + result.set_drops_artifact_from_ir(true); } if (!soname_not_regex_str.empty()) { - result->set_soname_not_regex_str(soname_not_regex_str); - result->set_drops_artifact_from_ir(true); + result.set_soname_not_regex_str(soname_not_regex_str); + result.set_drops_artifact_from_ir(true); } - return result; + suppr.reset(new file_suppression(result)); + return true; } /// Test if a given suppression specification is a file suppression