[v3,17/21] Refactor suppression property string parsing.

  This patch introduces a helper function to look up a key in an ini
configuration section and get a string value. All string look-ups now
use this.

There are no behavioural changes.

Signed-off-by: Giuliano Procida <gprocida@google.com>
---
 src/abg-suppression.cc | 198 ++++++++++++++++-------------------------
 1 file changed, 78 insertions(+), 120 deletions(-)
  

On Fri, Apr 24, 2020 at 10:21:28AM +0100, Giuliano Procida wrote:
>This patch introduces a helper function to look up a key in an ini
>configuration section and get a string value. All string look-ups now
>use this.
>
>There are no behavioural changes.
>
>Signed-off-by: Giuliano Procida <gprocida@google.com>
>---
> src/abg-suppression.cc | 198 ++++++++++++++++-------------------------
> 1 file changed, 78 insertions(+), 120 deletions(-)
>
>diff --git a/src/abg-suppression.cc b/src/abg-suppression.cc
>index 7d411caf..84c9b78f 100644
>--- a/src/abg-suppression.cc
>+++ b/src/abg-suppression.cc
>@@ -1594,6 +1594,31 @@ read_suppression_reach_kind(const string& input)
>     return type_suppression::DIRECT_REACH_KIND;
> }
>
>+/// Maybe fetch a property to a string.
>+///
>+/// Attempt to find a simple property in an ini section and copy it
>+/// to a string
>+///
>+/// @param section the ini section to look in
>+///
>+/// @param prop the property name
>+///
>+/// @param str the string to compile into
>+///
>+/// @return whether the property was found
>+static bool
>+maybe_get_string_prop(const ini::config::section& section,
>+                      const std::string& name,
>+                      std::string& str)

I would prefer a signature like

std::string get_string_prop(... section, ... name, std::string default = "");

In fact, this could be also

   template<typename T>
   T get_prop(... section, ... name, T default);

if prop->get_value() would be changed accordingly.

This does not play nicely with several keys to lookup though ...

Cheers,
Matthias

>+{
>+  ini::simple_property_sptr prop =
>+    is_simple_property(section.find_property(name));
>+  if (!prop)
>+    return false;
>+  str = prop->get_value()->as_string();
>+  return true;
>+}
>+
> /// Read a type suppression from an instance of ini::config::section
> /// and build a @ref type_suppression as a result.
> ///
>@@ -1609,18 +1634,12 @@ read_type_suppression(const ini::config::section& section)
>   if (section.get_name() != "suppress_type")
>     return result;
>
>-  ini::simple_property_sptr drop_artifact =
>-    is_simple_property(section.find_property("drop_artifact"));
>-  if (!drop_artifact)
>-    drop_artifact = is_simple_property(section.find_property("drop"));
>-
>-  string drop_artifact_str = drop_artifact
>-    ? drop_artifact->get_value()->as_string()
>-    : "";
>+  string drop_artifact_str;
>+  maybe_get_string_prop(section, "drop_artifact", drop_artifact_str)
>+    || maybe_get_string_prop(section, "drop", drop_artifact_str);
>
>-  ini::simple_property_sptr label =
>-    is_simple_property(section.find_property("label"));
>-  string label_str = label ? label->get_value()->as_string() : "";
>+  string label_str;
>+  maybe_get_string_prop(section, "label", label_str);
>
>   ini::simple_property_sptr file_name_regex_prop =
>     is_simple_property(section.find_property("file_name_regexp"));
>@@ -1662,11 +1681,8 @@ read_type_suppression(const ini::config::section& section)
>     name_not_regex =
>       regex::compile(name_not_regex_prop->get_value()->as_string());
>
>-  ini::simple_property_sptr name_prop =
>-    is_simple_property(section.find_property("name"));
>-  string name_str = name_prop
>-    ? name_prop->get_value()->as_string()
>-    : "";
>+  string name_str;
>+  maybe_get_string_prop(section, "name", name_str);
>
>   ini::property_sptr srcloc_not_in_prop =
>     section.find_property("source_location_not_in");
>@@ -1697,25 +1713,19 @@ read_type_suppression(const ini::config::section& section)
>     srcloc_not_regex =
>       regex::compile(srcloc_not_regexp_prop->get_value()->as_string());
>
>-  bool consider_type_kind = false;
>-  type_suppression::type_kind type_kind = type_suppression::UNKNOWN_TYPE_KIND;
>-  if (ini::simple_property_sptr type_kind_prop =
>-      is_simple_property(section.find_property("type_kind")))
>-    {
>-      consider_type_kind = true;
>-      type_kind =
>-	read_type_kind_string(type_kind_prop->get_value()->as_string());
>-    }
>+  std::string type_kind_str;
>+  bool consider_type_kind =
>+    maybe_get_string_prop(section, "type_kind", type_kind_str);
>+  type_suppression::type_kind type_kind = consider_type_kind
>+    ? read_type_kind_string(type_kind_str)
>+    : type_suppression::UNKNOWN_TYPE_KIND;
>
>-  bool consider_reach_kind = false;
>-  type_suppression::reach_kind reach_kind = type_suppression::DIRECT_REACH_KIND;
>-  if (ini::simple_property_sptr reach_kind_prop =
>-      is_simple_property(section.find_property("accessed_through")))
>-    {
>-      consider_reach_kind = true;
>-      reach_kind =
>-	read_suppression_reach_kind(reach_kind_prop->get_value()->as_string());
>-    }
>+  std::string reach_kind_str;
>+  bool consider_reach_kind =
>+      maybe_get_string_prop(section, "accessed_through", reach_kind_str);
>+  type_suppression::reach_kind reach_kind = consider_reach_kind
>+    ? read_suppression_reach_kind(reach_kind_str)
>+    : type_suppression::DIRECT_REACH_KIND;
>
>   // Support has_data_member_inserted_at
>   vector<type_suppression::insertion_range_sptr> insert_ranges;
>@@ -3175,26 +3185,15 @@ read_function_suppression(const ini::config::section& section)
>   if (section.get_name() != "suppress_function")
>     return result;
>
>-  ini::simple_property_sptr drop_artifact =
>-    is_simple_property(section.find_property("drop_artifact"));
>-  if (!drop_artifact)
>-    drop_artifact = is_simple_property(section.find_property("drop"));
>-
>-  string drop_artifact_str = drop_artifact
>-    ? drop_artifact->get_value()->as_string()
>-    : "";
>+  string drop_artifact_str;
>+  maybe_get_string_prop(section, "drop_artifact", drop_artifact_str)
>+    || maybe_get_string_prop(section, "drop", drop_artifact_str);
>
>-  ini::simple_property_sptr change_kind_prop =
>-    is_simple_property(section.find_property("change_kind"));
>-  string change_kind_str = change_kind_prop
>-    ? change_kind_prop->get_value()->as_string()
>-    : "";
>+  string change_kind_str;
>+  maybe_get_string_prop(section, "change_kind", change_kind_str);
>
>-  ini::simple_property_sptr label_prop =
>-    is_simple_property(section.find_property("label"));
>-  string label_str = label_prop
>-    ? label_prop->get_value()->as_string()
>-    : "";
>+  string label_str;
>+  maybe_get_string_prop(section, "label", label_str);
>
>   ini::simple_property_sptr file_name_regex_prop =
>     is_simple_property(section.find_property("file_name_regexp"));
>@@ -3223,11 +3222,8 @@ read_function_suppression(const ini::config::section& section)
>     soname_not_regex =
>       regex::compile(soname_not_regex_prop->get_value()->as_string());
>
>-  ini::simple_property_sptr name_prop =
>-    is_simple_property(section.find_property("name"));
>-  string name = name_prop
>-    ? name_prop->get_value()->as_string()
>-    : "";
>+  string name;
>+  maybe_get_string_prop(section, "name", name);
>
>   ini::simple_property_sptr name_regex_prop =
>     is_simple_property(section.find_property("name_regexp"));
>@@ -3242,11 +3238,8 @@ read_function_suppression(const ini::config::section& section)
>     name_not_regex =
>       regex::compile(name_not_regex_prop->get_value()->as_string());
>
>-  ini::simple_property_sptr return_type_name_prop =
>-    is_simple_property(section.find_property("return_type_name"));
>-  string return_type_name = return_type_name_prop
>-    ? return_type_name_prop->get_value()->as_string()
>-    : "";
>+  string return_type_name;
>+  maybe_get_string_prop(section, "return_type_name", return_type_name);
>
>   ini::simple_property_sptr return_type_regex_prop =
>     is_simple_property(section.find_property("return_type_regexp"));
>@@ -3255,11 +3248,8 @@ read_function_suppression(const ini::config::section& section)
>     return_type_regex =
>       regex::compile(return_type_regex_prop->get_value()->as_string());
>
>-  ini::simple_property_sptr sym_name_prop =
>-    is_simple_property(section.find_property("symbol_name"));
>-  string sym_name = sym_name_prop
>-    ? sym_name_prop->get_value()->as_string()
>-    : "";
>+  string sym_name;
>+  maybe_get_string_prop(section, "symbol_name", sym_name);
>
>   ini::simple_property_sptr sym_name_regex_prop =
>     is_simple_property(section.find_property("symbol_name_regexp"));
>@@ -3275,11 +3265,8 @@ read_function_suppression(const ini::config::section& section)
>     sym_name_not_regex =
>       regex::compile(sym_name_not_regex_prop->get_value()->as_string());
>
>-  ini::simple_property_sptr sym_ver_prop =
>-    is_simple_property(section.find_property("symbol_version"));
>-  string sym_version = sym_ver_prop
>-    ? sym_ver_prop->get_value()->as_string()
>-    : "";
>+  string sym_version;
>+  maybe_get_string_prop(section, "symbol_version", sym_version);
>
>   ini::simple_property_sptr sym_ver_regex_prop =
>     is_simple_property(section.find_property("symbol_version_regexp"));
>@@ -3288,11 +3275,8 @@ read_function_suppression(const ini::config::section& section)
>     sym_ver_regex =
>       regex::compile(sym_ver_regex_prop->get_value()->as_string());
>
>-  ini::simple_property_sptr allow_other_aliases_prop =
>-    is_simple_property(section.find_property("allow_other_aliases"));
>-  string allow_other_aliases = allow_other_aliases_prop
>-    ? allow_other_aliases_prop->get_value()->as_string()
>-    : "";
>+  string allow_other_aliases;
>+  maybe_get_string_prop(section, "allow_other_aliases", allow_other_aliases);
>
>   function_suppression::parameter_spec_sptr parm;
>   function_suppression::parameter_specs_type parms;
>@@ -4031,26 +4015,15 @@ read_variable_suppression(const ini::config::section& section)
>   if (section.get_name() != "suppress_variable")
>     return result;
>
>-  ini::simple_property_sptr drop_artifact =
>-    is_simple_property(section.find_property("drop_artifact"));
>-  if (!drop_artifact)
>-    drop_artifact = is_simple_property(section.find_property("drop"));
>-
>-  string drop_artifact_str = drop_artifact
>-    ? drop_artifact->get_value()->as_string()
>-    : "";
>+  string drop_artifact_str;
>+  maybe_get_string_prop(section, "drop_artifact", drop_artifact_str)
>+    || maybe_get_string_prop(section, "drop", drop_artifact_str);
>
>-  ini::simple_property_sptr change_kind_prop =
>-    is_simple_property(section.find_property("change_kind"));
>-  string change_kind_str = change_kind_prop
>-    ? change_kind_prop->get_value()->as_string()
>-    : "";
>+  string change_kind_str;
>+  maybe_get_string_prop(section, "change_kind", change_kind_str);
>
>-  ini::simple_property_sptr label_prop =
>-    is_simple_property(section.find_property("label"));
>-  string label_str = (label_prop
>-		      ? label_prop->get_value()->as_string()
>-		      : "");
>+  string label_str;
>+  maybe_get_string_prop(section, "label", label_str);
>
>   ini::simple_property_sptr file_name_regex_prop =
>     is_simple_property(section.find_property("file_name_regexp"));
>@@ -4079,11 +4052,8 @@ read_variable_suppression(const ini::config::section& section)
>     soname_not_regex =
>       regex::compile(soname_not_regex_prop->get_value()->as_string());
>
>-  ini::simple_property_sptr name_prop =
>-    is_simple_property(section.find_property("name"));
>-  string name_str = (name_prop
>-		     ? name_prop->get_value()->as_string()
>-		     : "");
>+  string name_str;
>+  maybe_get_string_prop(section, "name", name_str);
>
>   ini::simple_property_sptr name_regex_prop =
>     is_simple_property(section.find_property("name_regexp"));
>@@ -4098,11 +4068,8 @@ read_variable_suppression(const ini::config::section& section)
>     name_not_regex =
>       regex::compile(name_not_regex_prop->get_value()->as_string());
>
>-  ini::simple_property_sptr sym_name_prop =
>-    is_simple_property(section.find_property("symbol_name"));
>-  string symbol_name = (sym_name_prop
>-			? sym_name_prop->get_value()->as_string()
>-			: "");
>+  string symbol_name;
>+  maybe_get_string_prop(section, "symbol_name", symbol_name);
>
>   ini::simple_property_sptr sym_name_regex_prop =
>     is_simple_property(section.find_property("symbol_name_regexp"));
>@@ -4118,11 +4085,8 @@ read_variable_suppression(const ini::config::section& section)
>     symbol_name_not_regex =
>       regex::compile(sym_name_not_regex_prop->get_value()->as_string());
>
>-  ini::simple_property_sptr sym_version_prop =
>-    is_simple_property(section.find_property("symbol_version"));
>-  string symbol_version = sym_version_prop
>-    ? sym_version_prop->get_value()->as_string()
>-    : "";
>+  string symbol_version;
>+  maybe_get_string_prop(section, "symbol_version", symbol_version);
>
>   ini::simple_property_sptr sym_version_regex_prop =
>     is_simple_property(section.find_property("symbol_version_regexp"));
>@@ -4131,11 +4095,8 @@ read_variable_suppression(const ini::config::section& section)
>     symbol_version_regex =
>       regex::compile(sym_version_regex_prop->get_value()->as_string());
>
>-  ini::simple_property_sptr type_name_prop =
>-    is_simple_property(section.find_property("type_name"));
>-  string type_name_str = type_name_prop
>-    ? type_name_prop->get_value()->as_string()
>-    : "";
>+  string type_name_str;
>+  maybe_get_string_prop(section, "type_name", type_name_str);
>
>   ini::simple_property_sptr type_name_regex_prop =
>     is_simple_property(section.find_property("type_name_regexp"));
>@@ -4282,11 +4243,8 @@ read_file_suppression(const ini::config::section& section)
>   if (section.get_name() != "suppress_file")
>     return result;
>
>-  ini::simple_property_sptr label_prop =
>-    is_simple_property(section.find_property("label"));
>-  string label_str = (label_prop
>-		      ? label_prop->get_value()->as_string()
>-		      : "");
>+  string label_str;
>+  maybe_get_string_prop(section, "label", label_str);
>
>   ini::simple_property_sptr file_name_regex_prop =
>     is_simple_property(section.find_property("file_name_regexp"));
>-- 
>2.26.2.303.gf8c07b1a785-goog
>
  

Hi.

On Mon, 27 Apr 2020 at 13:17, Matthias Maennich <maennich@google.com> wrote:
>
> On Fri, Apr 24, 2020 at 10:21:28AM +0100, Giuliano Procida wrote:
> >This patch introduces a helper function to look up a key in an ini
> >configuration section and get a string value. All string look-ups now
> >use this.
> >
> >There are no behavioural changes.
> >
> >Signed-off-by: Giuliano Procida <gprocida@google.com>
> >---
> > src/abg-suppression.cc | 198 ++++++++++++++++-------------------------
> > 1 file changed, 78 insertions(+), 120 deletions(-)
> >
> >diff --git a/src/abg-suppression.cc b/src/abg-suppression.cc
> >index 7d411caf..84c9b78f 100644
> >--- a/src/abg-suppression.cc
> >+++ b/src/abg-suppression.cc
> >@@ -1594,6 +1594,31 @@ read_suppression_reach_kind(const string& input)
> >     return type_suppression::DIRECT_REACH_KIND;
> > }
> >
> >+/// Maybe fetch a property to a string.
> >+///
> >+/// Attempt to find a simple property in an ini section and copy it
> >+/// to a string
> >+///
> >+/// @param section the ini section to look in
> >+///
> >+/// @param prop the property name
> >+///
> >+/// @param str the string to compile into
> >+///
> >+/// @return whether the property was found
> >+static bool
> >+maybe_get_string_prop(const ini::config::section& section,
> >+                      const std::string& name,
> >+                      std::string& str)
>
> I would prefer a signature like
>
> std::string get_string_prop(... section, ... name, std::string default = "");
>
> In fact, this could be also
>
>    template<typename T>
>    T get_prop(... section, ... name, T default);
>
> if prop->get_value() would be changed accordingly.
>
> This does not play nicely with several keys to lookup though ...

maybe_get_string_prop and maybe_get_regex_prop were the obvious code
refactoring to do as there was a lot of repetition. From the
perspective of M commits later, they are the wrong choice.

In hindsight (!) I should have made a different choice as to how I
ordered my various changes. Something like this:

(this is for strings, something more involved for other types)

1. replace the code above with either

a) if a value is needed unconditionally (as a constructor argument)

string foo_str;
if (prop foo_prop = section.find_property("foo"))
  read_string(foo_prop, foo_str);

b) or conditionally:

if (prop prop = section.find_property("foo")) {
  string str;
  if (read_string(prop, str))
    suppression.set_foo(str);
}

also replacing code like this at the same time:

if (!foo_str.empty())
  suppression.set_foo(str);

2. Make the suppression constructors all default

This would mean switching all the code using pattern a) above to pattern b).

Default values are set by the default constructor, rather than being
scattered about the code.

...
...

N. Replace the find_property calls (and the special scan for
"parameter" properties) with something table-driven.

I can rework the patch series from this point along these lines.
Before I launch into it, let me know what you think! Thanks.

Giuliano.

> Cheers,
> Matthias
>
> >+{
> >+  ini::simple_property_sptr prop =
> >+    is_simple_property(section.find_property(name));
> >+  if (!prop)
> >+    return false;
> >+  str = prop->get_value()->as_string();
> >+  return true;
> >+}
> >+
> > /// Read a type suppression from an instance of ini::config::section
> > /// and build a @ref type_suppression as a result.
> > ///
> >@@ -1609,18 +1634,12 @@ read_type_suppression(const ini::config::section& section)
> >   if (section.get_name() != "suppress_type")
> >     return result;
> >
> >-  ini::simple_property_sptr drop_artifact =
> >-    is_simple_property(section.find_property("drop_artifact"));
> >-  if (!drop_artifact)
> >-    drop_artifact = is_simple_property(section.find_property("drop"));
> >-
> >-  string drop_artifact_str = drop_artifact
> >-    ? drop_artifact->get_value()->as_string()
> >-    : "";
> >+  string drop_artifact_str;
> >+  maybe_get_string_prop(section, "drop_artifact", drop_artifact_str)
> >+    || maybe_get_string_prop(section, "drop", drop_artifact_str);
> >
> >-  ini::simple_property_sptr label =
> >-    is_simple_property(section.find_property("label"));
> >-  string label_str = label ? label->get_value()->as_string() : "";
> >+  string label_str;
> >+  maybe_get_string_prop(section, "label", label_str);
> >
> >   ini::simple_property_sptr file_name_regex_prop =
> >     is_simple_property(section.find_property("file_name_regexp"));
> >@@ -1662,11 +1681,8 @@ read_type_suppression(const ini::config::section& section)
> >     name_not_regex =
> >       regex::compile(name_not_regex_prop->get_value()->as_string());
> >
> >-  ini::simple_property_sptr name_prop =
> >-    is_simple_property(section.find_property("name"));
> >-  string name_str = name_prop
> >-    ? name_prop->get_value()->as_string()
> >-    : "";
> >+  string name_str;
> >+  maybe_get_string_prop(section, "name", name_str);
> >
> >   ini::property_sptr srcloc_not_in_prop =
> >     section.find_property("source_location_not_in");
> >@@ -1697,25 +1713,19 @@ read_type_suppression(const ini::config::section& section)
> >     srcloc_not_regex =
> >       regex::compile(srcloc_not_regexp_prop->get_value()->as_string());
> >
> >-  bool consider_type_kind = false;
> >-  type_suppression::type_kind type_kind = type_suppression::UNKNOWN_TYPE_KIND;
> >-  if (ini::simple_property_sptr type_kind_prop =
> >-      is_simple_property(section.find_property("type_kind")))
> >-    {
> >-      consider_type_kind = true;
> >-      type_kind =
> >-      read_type_kind_string(type_kind_prop->get_value()->as_string());
> >-    }
> >+  std::string type_kind_str;
> >+  bool consider_type_kind =
> >+    maybe_get_string_prop(section, "type_kind", type_kind_str);
> >+  type_suppression::type_kind type_kind = consider_type_kind
> >+    ? read_type_kind_string(type_kind_str)
> >+    : type_suppression::UNKNOWN_TYPE_KIND;
> >
> >-  bool consider_reach_kind = false;
> >-  type_suppression::reach_kind reach_kind = type_suppression::DIRECT_REACH_KIND;
> >-  if (ini::simple_property_sptr reach_kind_prop =
> >-      is_simple_property(section.find_property("accessed_through")))
> >-    {
> >-      consider_reach_kind = true;
> >-      reach_kind =
> >-      read_suppression_reach_kind(reach_kind_prop->get_value()->as_string());
> >-    }
> >+  std::string reach_kind_str;
> >+  bool consider_reach_kind =
> >+      maybe_get_string_prop(section, "accessed_through", reach_kind_str);
> >+  type_suppression::reach_kind reach_kind = consider_reach_kind
> >+    ? read_suppression_reach_kind(reach_kind_str)
> >+    : type_suppression::DIRECT_REACH_KIND;
> >
> >   // Support has_data_member_inserted_at
> >   vector<type_suppression::insertion_range_sptr> insert_ranges;
> >@@ -3175,26 +3185,15 @@ read_function_suppression(const ini::config::section& section)
> >   if (section.get_name() != "suppress_function")
> >     return result;
> >
> >-  ini::simple_property_sptr drop_artifact =
> >-    is_simple_property(section.find_property("drop_artifact"));
> >-  if (!drop_artifact)
> >-    drop_artifact = is_simple_property(section.find_property("drop"));
> >-
> >-  string drop_artifact_str = drop_artifact
> >-    ? drop_artifact->get_value()->as_string()
> >-    : "";
> >+  string drop_artifact_str;
> >+  maybe_get_string_prop(section, "drop_artifact", drop_artifact_str)
> >+    || maybe_get_string_prop(section, "drop", drop_artifact_str);
> >
> >-  ini::simple_property_sptr change_kind_prop =
> >-    is_simple_property(section.find_property("change_kind"));
> >-  string change_kind_str = change_kind_prop
> >-    ? change_kind_prop->get_value()->as_string()
> >-    : "";
> >+  string change_kind_str;
> >+  maybe_get_string_prop(section, "change_kind", change_kind_str);
> >
> >-  ini::simple_property_sptr label_prop =
> >-    is_simple_property(section.find_property("label"));
> >-  string label_str = label_prop
> >-    ? label_prop->get_value()->as_string()
> >-    : "";
> >+  string label_str;
> >+  maybe_get_string_prop(section, "label", label_str);
> >
> >   ini::simple_property_sptr file_name_regex_prop =
> >     is_simple_property(section.find_property("file_name_regexp"));
> >@@ -3223,11 +3222,8 @@ read_function_suppression(const ini::config::section& section)
> >     soname_not_regex =
> >       regex::compile(soname_not_regex_prop->get_value()->as_string());
> >
> >-  ini::simple_property_sptr name_prop =
> >-    is_simple_property(section.find_property("name"));
> >-  string name = name_prop
> >-    ? name_prop->get_value()->as_string()
> >-    : "";
> >+  string name;
> >+  maybe_get_string_prop(section, "name", name);
> >
> >   ini::simple_property_sptr name_regex_prop =
> >     is_simple_property(section.find_property("name_regexp"));
> >@@ -3242,11 +3238,8 @@ read_function_suppression(const ini::config::section& section)
> >     name_not_regex =
> >       regex::compile(name_not_regex_prop->get_value()->as_string());
> >
> >-  ini::simple_property_sptr return_type_name_prop =
> >-    is_simple_property(section.find_property("return_type_name"));
> >-  string return_type_name = return_type_name_prop
> >-    ? return_type_name_prop->get_value()->as_string()
> >-    : "";
> >+  string return_type_name;
> >+  maybe_get_string_prop(section, "return_type_name", return_type_name);
> >
> >   ini::simple_property_sptr return_type_regex_prop =
> >     is_simple_property(section.find_property("return_type_regexp"));
> >@@ -3255,11 +3248,8 @@ read_function_suppression(const ini::config::section& section)
> >     return_type_regex =
> >       regex::compile(return_type_regex_prop->get_value()->as_string());
> >
> >-  ini::simple_property_sptr sym_name_prop =
> >-    is_simple_property(section.find_property("symbol_name"));
> >-  string sym_name = sym_name_prop
> >-    ? sym_name_prop->get_value()->as_string()
> >-    : "";
> >+  string sym_name;
> >+  maybe_get_string_prop(section, "symbol_name", sym_name);
> >
> >   ini::simple_property_sptr sym_name_regex_prop =
> >     is_simple_property(section.find_property("symbol_name_regexp"));
> >@@ -3275,11 +3265,8 @@ read_function_suppression(const ini::config::section& section)
> >     sym_name_not_regex =
> >       regex::compile(sym_name_not_regex_prop->get_value()->as_string());
> >
> >-  ini::simple_property_sptr sym_ver_prop =
> >-    is_simple_property(section.find_property("symbol_version"));
> >-  string sym_version = sym_ver_prop
> >-    ? sym_ver_prop->get_value()->as_string()
> >-    : "";
> >+  string sym_version;
> >+  maybe_get_string_prop(section, "symbol_version", sym_version);
> >
> >   ini::simple_property_sptr sym_ver_regex_prop =
> >     is_simple_property(section.find_property("symbol_version_regexp"));
> >@@ -3288,11 +3275,8 @@ read_function_suppression(const ini::config::section& section)
> >     sym_ver_regex =
> >       regex::compile(sym_ver_regex_prop->get_value()->as_string());
> >
> >-  ini::simple_property_sptr allow_other_aliases_prop =
> >-    is_simple_property(section.find_property("allow_other_aliases"));
> >-  string allow_other_aliases = allow_other_aliases_prop
> >-    ? allow_other_aliases_prop->get_value()->as_string()
> >-    : "";
> >+  string allow_other_aliases;
> >+  maybe_get_string_prop(section, "allow_other_aliases", allow_other_aliases);
> >
> >   function_suppression::parameter_spec_sptr parm;
> >   function_suppression::parameter_specs_type parms;
> >@@ -4031,26 +4015,15 @@ read_variable_suppression(const ini::config::section& section)
> >   if (section.get_name() != "suppress_variable")
> >     return result;
> >
> >-  ini::simple_property_sptr drop_artifact =
> >-    is_simple_property(section.find_property("drop_artifact"));
> >-  if (!drop_artifact)
> >-    drop_artifact = is_simple_property(section.find_property("drop"));
> >-
> >-  string drop_artifact_str = drop_artifact
> >-    ? drop_artifact->get_value()->as_string()
> >-    : "";
> >+  string drop_artifact_str;
> >+  maybe_get_string_prop(section, "drop_artifact", drop_artifact_str)
> >+    || maybe_get_string_prop(section, "drop", drop_artifact_str);
> >
> >-  ini::simple_property_sptr change_kind_prop =
> >-    is_simple_property(section.find_property("change_kind"));
> >-  string change_kind_str = change_kind_prop
> >-    ? change_kind_prop->get_value()->as_string()
> >-    : "";
> >+  string change_kind_str;
> >+  maybe_get_string_prop(section, "change_kind", change_kind_str);
> >
> >-  ini::simple_property_sptr label_prop =
> >-    is_simple_property(section.find_property("label"));
> >-  string label_str = (label_prop
> >-                    ? label_prop->get_value()->as_string()
> >-                    : "");
> >+  string label_str;
> >+  maybe_get_string_prop(section, "label", label_str);
> >
> >   ini::simple_property_sptr file_name_regex_prop =
> >     is_simple_property(section.find_property("file_name_regexp"));
> >@@ -4079,11 +4052,8 @@ read_variable_suppression(const ini::config::section& section)
> >     soname_not_regex =
> >       regex::compile(soname_not_regex_prop->get_value()->as_string());
> >
> >-  ini::simple_property_sptr name_prop =
> >-    is_simple_property(section.find_property("name"));
> >-  string name_str = (name_prop
> >-                   ? name_prop->get_value()->as_string()
> >-                   : "");
> >+  string name_str;
> >+  maybe_get_string_prop(section, "name", name_str);
> >
> >   ini::simple_property_sptr name_regex_prop =
> >     is_simple_property(section.find_property("name_regexp"));
> >@@ -4098,11 +4068,8 @@ read_variable_suppression(const ini::config::section& section)
> >     name_not_regex =
> >       regex::compile(name_not_regex_prop->get_value()->as_string());
> >
> >-  ini::simple_property_sptr sym_name_prop =
> >-    is_simple_property(section.find_property("symbol_name"));
> >-  string symbol_name = (sym_name_prop
> >-                      ? sym_name_prop->get_value()->as_string()
> >-                      : "");
> >+  string symbol_name;
> >+  maybe_get_string_prop(section, "symbol_name", symbol_name);
> >
> >   ini::simple_property_sptr sym_name_regex_prop =
> >     is_simple_property(section.find_property("symbol_name_regexp"));
> >@@ -4118,11 +4085,8 @@ read_variable_suppression(const ini::config::section& section)
> >     symbol_name_not_regex =
> >       regex::compile(sym_name_not_regex_prop->get_value()->as_string());
> >
> >-  ini::simple_property_sptr sym_version_prop =
> >-    is_simple_property(section.find_property("symbol_version"));
> >-  string symbol_version = sym_version_prop
> >-    ? sym_version_prop->get_value()->as_string()
> >-    : "";
> >+  string symbol_version;
> >+  maybe_get_string_prop(section, "symbol_version", symbol_version);
> >
> >   ini::simple_property_sptr sym_version_regex_prop =
> >     is_simple_property(section.find_property("symbol_version_regexp"));
> >@@ -4131,11 +4095,8 @@ read_variable_suppression(const ini::config::section& section)
> >     symbol_version_regex =
> >       regex::compile(sym_version_regex_prop->get_value()->as_string());
> >
> >-  ini::simple_property_sptr type_name_prop =
> >-    is_simple_property(section.find_property("type_name"));
> >-  string type_name_str = type_name_prop
> >-    ? type_name_prop->get_value()->as_string()
> >-    : "";
> >+  string type_name_str;
> >+  maybe_get_string_prop(section, "type_name", type_name_str);
> >
> >   ini::simple_property_sptr type_name_regex_prop =
> >     is_simple_property(section.find_property("type_name_regexp"));
> >@@ -4282,11 +4243,8 @@ read_file_suppression(const ini::config::section& section)
> >   if (section.get_name() != "suppress_file")
> >     return result;
> >
> >-  ini::simple_property_sptr label_prop =
> >-    is_simple_property(section.find_property("label"));
> >-  string label_str = (label_prop
> >-                    ? label_prop->get_value()->as_string()
> >-                    : "");
> >+  string label_str;
> >+  maybe_get_string_prop(section, "label", label_str);
> >
> >   ini::simple_property_sptr file_name_regex_prop =
> >     is_simple_property(section.find_property("file_name_regexp"));
> >--
> >2.26.2.303.gf8c07b1a785-goog
> >