diff mbox series

[v3,13/21] Move match methods from priv to suppression_base.

Message ID 20200424092132.150547-14-gprocida@google.com
State Superseded
Headers show
Series Simplify regex and suppression parsing. | expand

Commit Message

Giuliano Procida April 24, 2020, 9:21 a.m. UTC
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

Matthias Maennich April 27, 2020, 11:55 a.m. UTC | #1
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
>
diff mbox series

Patch

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