From: Giuliano Procida <gprocida@google.com>
To: libabigail@sourceware.org
Cc: dodji@seketeli.org, kernel-team@android.com, gprocida@google.com
Subject: [PATCH 4/7] Add read_*_suppression success/failure plumbing.
Date: Mon, 17 Aug 2020 10:38:16 +0100 [thread overview]
Message-ID: <20200817093819.172380-5-gprocida@google.com> (raw)
In-Reply-To: <20200817093819.172380-1-gprocida@google.com>
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 <gprocida@google.com>
---
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;
}
// <function_suppression stuff>
@@ -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;
}
// </function_suppression stuff>
@@ -4023,18 +4030,19 @@ operator|(variable_suppression::change_kind l,
(static_cast<unsigned>(l) | static_cast<unsigned>(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;
}
// </variable_suppression stuff>
@@ -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
--
2.28.0.220.ged08abb693-goog
next prev parent reply other threads:[~2020-08-17 9:38 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-08-17 9:38 [PATCH 0/7] Suppression parsing - preparatory work Giuliano Procida
2020-08-17 9:38 ` [PATCH 1/7] Add missing newlines to end of test files Giuliano Procida
2020-10-01 14:36 ` Dodji Seketeli
2020-08-17 9:38 ` [PATCH 2/7] Fix two wrongs in test suppression regex Giuliano Procida
2020-10-01 14:42 ` Dodji Seketeli
2020-08-17 9:38 ` [PATCH 3/7] Better suppression section parsing delegation Giuliano Procida
2020-10-01 15:56 ` Dodji Seketeli
2020-10-02 7:20 ` Giuliano Procida
2020-08-17 9:38 ` Giuliano Procida [this message]
2020-08-17 9:38 ` [PATCH 5/7] Add error handling to read_suppressions Giuliano Procida
2020-08-17 9:38 ` [PATCH 6/7] Default construct suppression types Giuliano Procida
2020-08-17 9:38 ` [PATCH 7/7] Refresh getter/setter comments Giuliano Procida
2020-10-02 7:25 ` [PATCH 0/7] Suppression parsing - preparatory work Dodji Seketeli
2020-10-06 20:19 ` Giuliano Procida
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200817093819.172380-5-gprocida@google.com \
--to=gprocida@google.com \
--cc=dodji@seketeli.org \
--cc=kernel-team@android.com \
--cc=libabigail@sourceware.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).