[v3,13/21] Move match methods from priv to suppression_base.
Commit Message
The suppression_base::priv methods matches_soname and
matches_binary_name do not require special access to the priv class
and can be moved to the main suppression_base class and use its
getters.
This is a step towards simplifying the suppression type priv classes
to the point they are simple data containers.
There are no behavioural changes.
* include/abg-suppression.h
(suppression_base::matches_soname): Add declaration.
(suppression_base::matches_binary_name): Add declaration.
(suppression_base): Remove friend declarations of
suppression_matches_soname and
suppression_matches_soname_or_filename.
* src/abg-dwarf-reader.cc (suppression_can_match): Call
matches_soname and matches_binary_name on suppression values
directly.
* src/abg-reader.cc (suppression_can_match): Call
matches_soname and matches_binary_name on suppression values
directly.
* src/abg-suppression-priv.h
(suppression_base::priv::matches_soname): Remove method.
(suppression_base::priv::matches_binary_name): Remove method.
* src/abg-suppression.cc
(suppression_base::matches_soname): Add definition.
(suppression_base::matches_binary_name): Add definition.
(names_of_binaries_match): Call matches_binary_name on
suppression values directly. (suppression_matches_soname):
Call matches_soname on suppression values directly.
(suppression_matches_soname_or_filename): Call
matches_binary_name on suppression values directly.
Signed-off-by: Giuliano Procida <gprocida@google.com>
---
include/abg-suppression.h | 15 +++-----
src/abg-dwarf-reader.cc | 4 +-
src/abg-reader.cc | 4 +-
src/abg-suppression-priv.h | 71 +----------------------------------
src/abg-suppression.cc | 76 ++++++++++++++++++++++++++++++++++++--
5 files changed, 83 insertions(+), 87 deletions(-)
Comments
On Fri, Apr 24, 2020 at 10:21:24AM +0100, Giuliano Procida wrote:
>The suppression_base::priv methods matches_soname and
>matches_binary_name do not require special access to the priv class
>and can be moved to the main suppression_base class and use its
>getters.
>
>This is a step towards simplifying the suppression type priv classes
>to the point they are simple data containers.
>
>There are no behavioural changes.
>
> * include/abg-suppression.h
> (suppression_base::matches_soname): Add declaration.
> (suppression_base::matches_binary_name): Add declaration.
> (suppression_base): Remove friend declarations of
> suppression_matches_soname and
> suppression_matches_soname_or_filename.
> * src/abg-dwarf-reader.cc (suppression_can_match): Call
> matches_soname and matches_binary_name on suppression values
> directly.
> * src/abg-reader.cc (suppression_can_match): Call
> matches_soname and matches_binary_name on suppression values
> directly.
> * src/abg-suppression-priv.h
> (suppression_base::priv::matches_soname): Remove method.
> (suppression_base::priv::matches_binary_name): Remove method.
> * src/abg-suppression.cc
> (suppression_base::matches_soname): Add definition.
> (suppression_base::matches_binary_name): Add definition.
> (names_of_binaries_match): Call matches_binary_name on
> suppression values directly. (suppression_matches_soname):
> Call matches_soname on suppression values directly.
> (suppression_matches_soname_or_filename): Call
> matches_binary_name on suppression values directly.
>
>Signed-off-by: Giuliano Procida <gprocida@google.com>
Reviewed-by: Matthias Maennich <maennich@google.com>
Cheers,
Matthias
>---
> include/abg-suppression.h | 15 +++-----
> src/abg-dwarf-reader.cc | 4 +-
> src/abg-reader.cc | 4 +-
> src/abg-suppression-priv.h | 71 +----------------------------------
> src/abg-suppression.cc | 76 ++++++++++++++++++++++++++++++++++++--
> 5 files changed, 83 insertions(+), 87 deletions(-)
>
>diff --git a/include/abg-suppression.h b/include/abg-suppression.h
>index 1869cbe5..02e77ea9 100644
>--- a/include/abg-suppression.h
>+++ b/include/abg-suppression.h
>@@ -116,19 +116,16 @@ public:
> bool
> has_soname_related_property() const;
>
>+ bool
>+ matches_soname(const string& soname) const;
>+
>+ bool
>+ matches_binary_name(const string& binary_name) const;
>+
> virtual bool
> suppresses_diff(const diff*) const = 0;
>
> virtual ~suppression_base();
>-
>- friend bool
>- suppression_matches_soname(const string& soname,
>- const suppression_base& suppr);
>-
>- friend bool
>- suppression_matches_soname_or_filename(const string& soname,
>- const string& filename,
>- const suppression_base& suppr);
> }; // end class suppression_base
>
> void
>diff --git a/src/abg-dwarf-reader.cc b/src/abg-dwarf-reader.cc
>index 850281ad..c9e6f918 100644
>--- a/src/abg-dwarf-reader.cc
>+++ b/src/abg-dwarf-reader.cc
>@@ -7723,14 +7723,14 @@ public:
> bool
> suppression_can_match(const suppr::suppression_base& s) const
> {
>- if (!s.priv_->matches_soname(dt_soname()))
>+ if (!s.matches_soname(dt_soname()))
> if (s.has_soname_related_property())
> // The suppression has some SONAME related properties, but
> // none of them match the SONAME of the current binary. So
> // the suppression cannot match the current binary.
> return false;
>
>- if (!s.priv_->matches_binary_name(elf_path()))
>+ if (!s.matches_binary_name(elf_path()))
> if (s.has_file_name_related_property())
> // The suppression has some file_name related properties, but
> // none of them match the file name of the current binary. So
>diff --git a/src/abg-reader.cc b/src/abg-reader.cc
>index dcaa27e1..5e0b97e0 100644
>--- a/src/abg-reader.cc
>+++ b/src/abg-reader.cc
>@@ -999,14 +999,14 @@ public:
> {
> corpus_sptr corp = get_corpus();
>
>- if (!s.priv_->matches_soname(corp->get_soname()))
>+ if (!s.matches_soname(corp->get_soname()))
> if (s.has_soname_related_property())
> // The suppression has some SONAME related properties, but
> // none of them match the SONAME of the current binary. So
> // the suppression cannot match the current binary.
> return false;
>
>- if (!s.priv_->matches_binary_name(corp->get_path()))
>+ if (!s.matches_binary_name(corp->get_path()))
> if (s.has_file_name_related_property())
> // The suppression has some file_name related properties, but
> // none of them match the file name of the current binary. So
>diff --git a/src/abg-suppression-priv.h b/src/abg-suppression-priv.h
>index 263052d4..5c06d5d0 100644
>--- a/src/abg-suppression-priv.h
>+++ b/src/abg-suppression-priv.h
>@@ -107,76 +107,7 @@ public:
> {
> return soname_not_regex_;
> }
>-
>- /// Test if the current suppression matches a given SONAME.
>- ///
>- /// @param soname the SONAME to consider.
>- ///
>- /// @return true iff the suppression matches the SONAME denoted by
>- /// @p soname.
>- ///
>- /// Note that if the suppression contains no property that is
>- /// related to SONAMEs, the function returns false.
>- bool
>- matches_soname(const string& soname) const
>- {
>- bool has_regexp = false;
>- if (regex::regex_t_sptr regexp = get_soname_regex())
>- {
>- has_regexp = true;
>- if (!regex::match(regexp, soname))
>- return false;
>- }
>-
>- if (regex::regex_t_sptr regexp = get_soname_not_regex())
>- {
>- has_regexp = true;
>- if (regex::match(regexp, soname))
>- return false;
>- }
>-
>- if (!has_regexp)
>- return false;
>-
>- return true;
>- }
>-
>- /// Test if the current suppression matches the full file path to a
>- /// given binary.
>- ///
>- /// @param binary_name the full path to the binary.
>- ///
>- /// @return true iff the suppression matches the path denoted by @p
>- /// binary_name.
>- ///
>- /// Note that if the suppression contains no property that is
>- /// related to file name, the function returns false.
>- bool
>- matches_binary_name(const string& binary_name) const
>- {
>- bool has_regexp = false;
>-
>- if (regex::regex_t_sptr regexp = get_file_name_regex())
>- {
>- has_regexp = true;
>- if (!regex::match(regexp, binary_name))
>- return false;
>- }
>-
>- if (regex::regex_t_sptr regexp = get_file_name_not_regex())
>- {
>- has_regexp = true;
>- if (regex::match(regexp, binary_name))
>- return false;
>- }
>-
>- if (!has_regexp)
>- return false;
>-
>- return true;
>- }
>-
>-}; // end clas suppression_base::priv
>+}; // end class suppression_base::priv
>
> // </suppression_base stuff>
>
>diff --git a/src/abg-suppression.cc b/src/abg-suppression.cc
>index 7e501c45..191c65c6 100644
>--- a/src/abg-suppression.cc
>+++ b/src/abg-suppression.cc
>@@ -252,6 +252,74 @@ suppression_base::has_soname_related_property() const
> return get_soname_regex() || get_soname_not_regex();
> }
>
>+/// Test if the current suppression matches a given SONAME.
>+///
>+/// @param soname the SONAME to consider.
>+///
>+/// @return true iff the suppression matches the SONAME denoted by
>+/// @p soname.
>+///
>+/// Note that if the suppression contains no property that is
>+/// related to SONAMEs, the function returns false.
>+bool
>+suppression_base::matches_soname(const string& soname) const
>+{
>+ bool has_regexp = false;
>+ if (regex::regex_t_sptr regexp = get_soname_regex())
>+ {
>+ has_regexp = true;
>+ if (!regex::match(regexp, soname))
>+ return false;
>+ }
>+
>+ if (regex::regex_t_sptr regexp = get_soname_not_regex())
>+ {
>+ has_regexp = true;
>+ if (regex::match(regexp, soname))
>+ return false;
>+ }
>+
>+ if (!has_regexp)
>+ return false;
>+
>+ return true;
>+}
>+
>+/// Test if the current suppression matches the full file path to a
>+/// given binary.
>+///
>+/// @param binary_name the full path to the binary.
>+///
>+/// @return true iff the suppression matches the path denoted by @p
>+/// binary_name.
>+///
>+/// Note that if the suppression contains no property that is
>+/// related to file name, the function returns false.
>+bool
>+suppression_base::matches_binary_name(const string& binary_name) const
>+{
>+ bool has_regexp = false;
>+
>+ if (regex::regex_t_sptr regexp = get_file_name_regex())
>+ {
>+ has_regexp = true;
>+ if (!regex::match(regexp, binary_name))
>+ return false;
>+ }
>+
>+ if (regex::regex_t_sptr regexp = get_file_name_not_regex())
>+ {
>+ has_regexp = true;
>+ if (regex::match(regexp, binary_name))
>+ return false;
>+ }
>+
>+ if (!has_regexp)
>+ return false;
>+
>+ return true;
>+}
>+
> /// Check if the SONAMEs of the two binaries being compared match the
> /// content of the properties "soname_regexp" and "soname_not_regexp"
> /// of the current suppression specification.
>@@ -305,8 +373,8 @@ names_of_binaries_match(const suppression_base& suppr,
> if (!suppr.has_file_name_related_property())
> return false;
>
>- if (!suppr.priv_->matches_binary_name(first_binary_path)
>- && !suppr.priv_->matches_binary_name(second_binary_path))
>+ if (!suppr.matches_binary_name(first_binary_path)
>+ && !suppr.matches_binary_name(second_binary_path))
> return false;
>
> return true;
>@@ -4330,7 +4398,7 @@ bool
> suppression_matches_soname(const string& soname,
> const suppression_base& suppr)
> {
>- return suppr.priv_->matches_soname(soname);
>+ return suppr.matches_soname(soname);
> }
>
> /// Test if a given SONAME or file name is matched by a given
>@@ -4350,7 +4418,7 @@ suppression_matches_soname_or_filename(const string& soname,
> const suppression_base& suppr)
> {
> return (suppression_matches_soname(soname, suppr)
>- || suppr.priv_->matches_binary_name(filename));
>+ || suppr.matches_binary_name(filename));
> }
>
> /// @return the name of the artificial private type suppression
>--
>2.26.2.303.gf8c07b1a785-goog
>
@@ -116,19 +116,16 @@ public:
bool
has_soname_related_property() const;
+ bool
+ matches_soname(const string& soname) const;
+
+ bool
+ matches_binary_name(const string& binary_name) const;
+
virtual bool
suppresses_diff(const diff*) const = 0;
virtual ~suppression_base();
-
- friend bool
- suppression_matches_soname(const string& soname,
- const suppression_base& suppr);
-
- friend bool
- suppression_matches_soname_or_filename(const string& soname,
- const string& filename,
- const suppression_base& suppr);
}; // end class suppression_base
void
@@ -7723,14 +7723,14 @@ public:
bool
suppression_can_match(const suppr::suppression_base& s) const
{
- if (!s.priv_->matches_soname(dt_soname()))
+ if (!s.matches_soname(dt_soname()))
if (s.has_soname_related_property())
// The suppression has some SONAME related properties, but
// none of them match the SONAME of the current binary. So
// the suppression cannot match the current binary.
return false;
- if (!s.priv_->matches_binary_name(elf_path()))
+ if (!s.matches_binary_name(elf_path()))
if (s.has_file_name_related_property())
// The suppression has some file_name related properties, but
// none of them match the file name of the current binary. So
@@ -999,14 +999,14 @@ public:
{
corpus_sptr corp = get_corpus();
- if (!s.priv_->matches_soname(corp->get_soname()))
+ if (!s.matches_soname(corp->get_soname()))
if (s.has_soname_related_property())
// The suppression has some SONAME related properties, but
// none of them match the SONAME of the current binary. So
// the suppression cannot match the current binary.
return false;
- if (!s.priv_->matches_binary_name(corp->get_path()))
+ if (!s.matches_binary_name(corp->get_path()))
if (s.has_file_name_related_property())
// The suppression has some file_name related properties, but
// none of them match the file name of the current binary. So
@@ -107,76 +107,7 @@ public:
{
return soname_not_regex_;
}
-
- /// Test if the current suppression matches a given SONAME.
- ///
- /// @param soname the SONAME to consider.
- ///
- /// @return true iff the suppression matches the SONAME denoted by
- /// @p soname.
- ///
- /// Note that if the suppression contains no property that is
- /// related to SONAMEs, the function returns false.
- bool
- matches_soname(const string& soname) const
- {
- bool has_regexp = false;
- if (regex::regex_t_sptr regexp = get_soname_regex())
- {
- has_regexp = true;
- if (!regex::match(regexp, soname))
- return false;
- }
-
- if (regex::regex_t_sptr regexp = get_soname_not_regex())
- {
- has_regexp = true;
- if (regex::match(regexp, soname))
- return false;
- }
-
- if (!has_regexp)
- return false;
-
- return true;
- }
-
- /// Test if the current suppression matches the full file path to a
- /// given binary.
- ///
- /// @param binary_name the full path to the binary.
- ///
- /// @return true iff the suppression matches the path denoted by @p
- /// binary_name.
- ///
- /// Note that if the suppression contains no property that is
- /// related to file name, the function returns false.
- bool
- matches_binary_name(const string& binary_name) const
- {
- bool has_regexp = false;
-
- if (regex::regex_t_sptr regexp = get_file_name_regex())
- {
- has_regexp = true;
- if (!regex::match(regexp, binary_name))
- return false;
- }
-
- if (regex::regex_t_sptr regexp = get_file_name_not_regex())
- {
- has_regexp = true;
- if (regex::match(regexp, binary_name))
- return false;
- }
-
- if (!has_regexp)
- return false;
-
- return true;
- }
-
-}; // end clas suppression_base::priv
+}; // end class suppression_base::priv
// </suppression_base stuff>
@@ -252,6 +252,74 @@ suppression_base::has_soname_related_property() const
return get_soname_regex() || get_soname_not_regex();
}
+/// Test if the current suppression matches a given SONAME.
+///
+/// @param soname the SONAME to consider.
+///
+/// @return true iff the suppression matches the SONAME denoted by
+/// @p soname.
+///
+/// Note that if the suppression contains no property that is
+/// related to SONAMEs, the function returns false.
+bool
+suppression_base::matches_soname(const string& soname) const
+{
+ bool has_regexp = false;
+ if (regex::regex_t_sptr regexp = get_soname_regex())
+ {
+ has_regexp = true;
+ if (!regex::match(regexp, soname))
+ return false;
+ }
+
+ if (regex::regex_t_sptr regexp = get_soname_not_regex())
+ {
+ has_regexp = true;
+ if (regex::match(regexp, soname))
+ return false;
+ }
+
+ if (!has_regexp)
+ return false;
+
+ return true;
+}
+
+/// Test if the current suppression matches the full file path to a
+/// given binary.
+///
+/// @param binary_name the full path to the binary.
+///
+/// @return true iff the suppression matches the path denoted by @p
+/// binary_name.
+///
+/// Note that if the suppression contains no property that is
+/// related to file name, the function returns false.
+bool
+suppression_base::matches_binary_name(const string& binary_name) const
+{
+ bool has_regexp = false;
+
+ if (regex::regex_t_sptr regexp = get_file_name_regex())
+ {
+ has_regexp = true;
+ if (!regex::match(regexp, binary_name))
+ return false;
+ }
+
+ if (regex::regex_t_sptr regexp = get_file_name_not_regex())
+ {
+ has_regexp = true;
+ if (regex::match(regexp, binary_name))
+ return false;
+ }
+
+ if (!has_regexp)
+ return false;
+
+ return true;
+}
+
/// Check if the SONAMEs of the two binaries being compared match the
/// content of the properties "soname_regexp" and "soname_not_regexp"
/// of the current suppression specification.
@@ -305,8 +373,8 @@ names_of_binaries_match(const suppression_base& suppr,
if (!suppr.has_file_name_related_property())
return false;
- if (!suppr.priv_->matches_binary_name(first_binary_path)
- && !suppr.priv_->matches_binary_name(second_binary_path))
+ if (!suppr.matches_binary_name(first_binary_path)
+ && !suppr.matches_binary_name(second_binary_path))
return false;
return true;
@@ -4330,7 +4398,7 @@ bool
suppression_matches_soname(const string& soname,
const suppression_base& suppr)
{
- return suppr.priv_->matches_soname(soname);
+ return suppr.matches_soname(soname);
}
/// Test if a given SONAME or file name is matched by a given
@@ -4350,7 +4418,7 @@ suppression_matches_soname_or_filename(const string& soname,
const suppression_base& suppr)
{
return (suppression_matches_soname(soname, suppr)
- || suppr.priv_->matches_binary_name(filename));
+ || suppr.matches_binary_name(filename));
}
/// @return the name of the artificial private type suppression