[v3,16/21] abg-tools-utils.cc: Assert generated regexes OK.

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

Commit Message

Giuliano Procida April 24, 2020, 9:21 a.m. UTC
  There are a couple of places where regexes are generated interally.
Assert they compile OK.

This is just paranoia. There should be no behavioural changes.

	* src/abg-tools-utils.cc (handle_file_entry): Assert
	internally-generated regex compiles.
	(gen_suppr_spec_from_kernel_abi_whitelists): Assert
	internally-generated regex compiles.

Signed-off-by: Giuliano Procida <gprocida@google.com>
---
 src/abg-tools-utils.cc | 2 ++
 1 file changed, 2 insertions(+)
  

Comments

Matthias Männich April 27, 2020, 12:08 p.m. UTC | #1
On Fri, Apr 24, 2020 at 10:21:27AM +0100, Giuliano Procida wrote:
>There are a couple of places where regexes are generated interally.
                                                              ^^^ typo
>Assert they compile OK.
>
>This is just paranoia. There should be no behavioural changes.
>
>	* src/abg-tools-utils.cc (handle_file_entry): Assert
>	internally-generated regex compiles.
>	(gen_suppr_spec_from_kernel_abi_whitelists): Assert
>	internally-generated regex compiles.
>
>Signed-off-by: Giuliano Procida <gprocida@google.com>

Reviewed-by: Matthias Maennich <maennich@google.com>

Cheers,
Matthias

>---
> src/abg-tools-utils.cc | 2 ++
> 1 file changed, 2 insertions(+)
>
>diff --git a/src/abg-tools-utils.cc b/src/abg-tools-utils.cc
>index fe0de750..3af9fc49 100644
>--- a/src/abg-tools-utils.cc
>+++ b/src/abg-tools-utils.cc
>@@ -1818,6 +1818,7 @@ handle_file_entry(const string& file_path,
>       // Types that are defined in system headers are usually
>       // OK to be considered as public types.
>       regex::regex_t_sptr headers_regex = regex::compile("^/usr/include/");
>+      ABG_ASSERT(headers_regex);
>       suppr->set_source_location_to_keep_regex(headers_regex);
>       suppr->set_is_artificial(true);
>     }
>@@ -2008,6 +2009,7 @@ gen_suppr_spec_from_kernel_abi_whitelists
>       // the function and variable names expressed in the white list.
>       regex::regex_t_sptr regex =
> 	regex::compile(regex::generate_from_strings(whitelisted_names));
>+      ABG_ASSERT(regex);
>
>       // Build a suppression specification which *keeps* functions
>       // whose ELF symbols match the regular expression contained
>-- 
>2.26.2.303.gf8c07b1a785-goog
>
  
Giuliano Procida April 27, 2020, 4:21 p.m. UTC | #2
Hi.

On Mon, 27 Apr 2020 at 13:08, Matthias Maennich <maennich@google.com> wrote:
>
> On Fri, Apr 24, 2020 at 10:21:27AM +0100, Giuliano Procida wrote:
> >There are a couple of places where regexes are generated interally.
>                                                               ^^^ typo

Fixed.

Thank you!

> >Assert they compile OK.
> >
> >This is just paranoia. There should be no behavioural changes.
> >
> >       * src/abg-tools-utils.cc (handle_file_entry): Assert
> >       internally-generated regex compiles.
> >       (gen_suppr_spec_from_kernel_abi_whitelists): Assert
> >       internally-generated regex compiles.
> >
> >Signed-off-by: Giuliano Procida <gprocida@google.com>
>
> Reviewed-by: Matthias Maennich <maennich@google.com>
>
> Cheers,
> Matthias
>
> >---
> > src/abg-tools-utils.cc | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> >diff --git a/src/abg-tools-utils.cc b/src/abg-tools-utils.cc
> >index fe0de750..3af9fc49 100644
> >--- a/src/abg-tools-utils.cc
> >+++ b/src/abg-tools-utils.cc
> >@@ -1818,6 +1818,7 @@ handle_file_entry(const string& file_path,
> >       // Types that are defined in system headers are usually
> >       // OK to be considered as public types.
> >       regex::regex_t_sptr headers_regex = regex::compile("^/usr/include/");
> >+      ABG_ASSERT(headers_regex);
> >       suppr->set_source_location_to_keep_regex(headers_regex);
> >       suppr->set_is_artificial(true);
> >     }
> >@@ -2008,6 +2009,7 @@ gen_suppr_spec_from_kernel_abi_whitelists
> >       // the function and variable names expressed in the white list.
> >       regex::regex_t_sptr regex =
> >       regex::compile(regex::generate_from_strings(whitelisted_names));
> >+      ABG_ASSERT(regex);
> >
> >       // Build a suppression specification which *keeps* functions
> >       // whose ELF symbols match the regular expression contained
> >--
> >2.26.2.303.gf8c07b1a785-goog
> >
  

Patch

diff --git a/src/abg-tools-utils.cc b/src/abg-tools-utils.cc
index fe0de750..3af9fc49 100644
--- a/src/abg-tools-utils.cc
+++ b/src/abg-tools-utils.cc
@@ -1818,6 +1818,7 @@  handle_file_entry(const string& file_path,
       // Types that are defined in system headers are usually
       // OK to be considered as public types.
       regex::regex_t_sptr headers_regex = regex::compile("^/usr/include/");
+      ABG_ASSERT(headers_regex);
       suppr->set_source_location_to_keep_regex(headers_regex);
       suppr->set_is_artificial(true);
     }
@@ -2008,6 +2009,7 @@  gen_suppr_spec_from_kernel_abi_whitelists
       // the function and variable names expressed in the white list.
       regex::regex_t_sptr regex =
 	regex::compile(regex::generate_from_strings(whitelisted_names));
+      ABG_ASSERT(regex);
 
       // Build a suppression specification which *keeps* functions
       // whose ELF symbols match the regular expression contained