(trafficserver) 01/01: Added more edge case tests Checking against nullptr in conditionals Moved cmake pkgconfig test to top level CMakeLists.txt

[bcall]

This is an automated email from the ASF dual-hosted git repository.

bcall pushed a commit to branch regex_interface
in repository https://gitbox.apache.org/repos/asf/trafficserver.git

commit 3c4890d21230530d5ec5a99934f15710f27e5c86
Author: Bryan Call <bc...@apache.org>
AuthorDate: Wed Feb 28 14:07:27 2024 -0800

    Added more edge case tests
    Checking against nullptr in conditionals
    Moved cmake pkgconfig test to top level CMakeLists.txt
---
 CMakeLists.txt                      |  2 +-
 include/tsutil/Regex.h              |  4 ----
 src/tsutil/CMakeLists.txt           |  2 --
 src/tsutil/Regex.cc                 | 18 +++++++++---------
 src/tsutil/unit_tests/test_Regex.cc | 29 +++++++++++++++++++++++------
 5 files changed, 33 insertions(+), 22 deletions(-)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 920d15835e..3a8f031fc2 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -258,7 +258,7 @@ if(LibLZMA_FOUND)
 endif()
 
 find_package(PCRE REQUIRED)
-find_package(PCRE2 COMPONENTS 8BIT)
+pkg_check_modules(PCRE2 REQUIRED IMPORTED_TARGET libpcre2-8)
 
 include(CheckOpenSSLIsBoringSSL)
 include(CheckOpenSSLIsQuictls)
diff --git a/include/tsutil/Regex.h b/include/tsutil/Regex.h
index 75a27c142e..c4ca8feb03 100644
--- a/include/tsutil/Regex.h
+++ b/include/tsutil/Regex.h
@@ -29,11 +29,7 @@
 #include <memory>
 
 #define PCRE2_CODE_UNIT_WIDTH 8
-#if __has_include(<pcre2/pcre2.h>)
-#include <pcre2/pcre2.h>
-#else
 #include <pcre2.h>
-#endif
 
 /// @brief Match flags for regular expression evaluation.
 enum REFlags {
diff --git a/src/tsutil/CMakeLists.txt b/src/tsutil/CMakeLists.txt
index c2538fabfb..a747431daa 100644
--- a/src/tsutil/CMakeLists.txt
+++ b/src/tsutil/CMakeLists.txt
@@ -51,8 +51,6 @@ add_library(
   Regex.cc
 )
 
-pkg_check_modules(PCRE2 REQUIRED IMPORTED_TARGET libpcre2-8)
-
 add_library(ts::tsutil ALIAS tsutil)
 set_target_properties(tsutil PROPERTIES POSITION_INDEPENDENT_CODE TRUE 
PUBLIC_HEADER "${TSUTIL_PUBLIC_HEADERS}")
 target_link_libraries(tsutil PUBLIC libswoc::libswoc yaml-cpp::yaml-cpp 
PkgConfig::PCRE2)
diff --git a/src/tsutil/Regex.cc b/src/tsutil/Regex.cc
index e077e30886..7ad756fac4 100644
--- a/src/tsutil/Regex.cc
+++ b/src/tsutil/Regex.cc
@@ -57,16 +57,16 @@ public:
   }
   ~RegexContext()
   {
-    if (_general_context) {
+    if (_general_context != nullptr) {
       pcre2_general_context_free(_general_context);
     }
-    if (_compile_context) {
+    if (_compile_context != nullptr) {
       pcre2_compile_context_free(_compile_context);
     }
-    if (_match_context) {
+    if (_match_context != nullptr) {
       pcre2_match_context_free(_match_context);
     }
-    if (_jit_stack) {
+    if (_jit_stack != nullptr) {
       pcre2_jit_stack_free(_jit_stack);
     }
   }
@@ -143,7 +143,7 @@ RegexMatches::malloc(size_t size, void *caller)
 //----------------------------------------------------------------------------
 RegexMatches::~RegexMatches()
 {
-  if (_match_data) {
+  if (_match_data == nullptr) {
     pcre2_match_data_free(_match_data);
   }
 }
@@ -206,7 +206,7 @@ Regex::Regex(Regex &&that) noexcept
 //----------------------------------------------------------------------------
 Regex::~Regex()
 {
-  if (_code) {
+  if (_code == nullptr) {
     pcre2_code_free(_code);
   }
 }
@@ -225,7 +225,7 @@ Regex::compile(std::string_view pattern, uint32_t flags)
 bool
 Regex::compile(std::string_view pattern, std::string &error, int &erroroffset, 
uint32_t flags)
 {
-  if (_code) {
+  if (_code != nullptr) {
     pcre2_code_free(_code);
   }
   PCRE2_SIZE error_offset;
@@ -252,7 +252,7 @@ Regex::compile(std::string_view pattern, std::string 
&error, int &erroroffset, u
 bool
 Regex::exec(std::string_view subject) const
 {
-  if (!_code) {
+  if (_code == nullptr) {
     return false;
   }
   RegexMatches matches;
@@ -265,7 +265,7 @@ Regex::exec(std::string_view subject) const
 int32_t
 Regex::exec(std::string_view subject, RegexMatches &matches) const
 {
-  if (!_code) {
+  if (_code == nullptr) {
     return 0;
   }
   int count = pcre2_match(_code, reinterpret_cast<PCRE2_SPTR>(subject.data()), 
subject.size(), 0, 0, matches.get_match_data(),
diff --git a/src/tsutil/unit_tests/test_Regex.cc 
b/src/tsutil/unit_tests/test_Regex.cc
index e2332272f6..f17d2b17c8 100644
--- a/src/tsutil/unit_tests/test_Regex.cc
+++ b/src/tsutil/unit_tests/test_Regex.cc
@@ -122,12 +122,6 @@ TEST_CASE("Regex", "[libts][Regex]")
     }
   }
 
-  // test for invalid regular expression
-  {
-    Regex r;
-    REQUIRE(r.compile(R"((\d+)", RE_CASE_INSENSITIVE) == false);
-  }
-
   // test getting submatches with operator[]
   for (auto &item : submatch_test_data) {
     Regex r;
@@ -162,4 +156,27 @@ TEST_CASE("Regex", "[libts][Regex]")
       }
     }
   }
+
+  // test for invalid regular expression
+  {
+    Regex r;
+    REQUIRE(r.compile(R"((\d+)", RE_CASE_INSENSITIVE) == false);
+  }
+
+  // test for not compiling regular expression
+  {
+    Regex r;
+    RegexMatches matches;
+    REQUIRE(r.exec("foo") == false);
+    REQUIRE(r.exec("foo", matches) == 0);
+  }
+
+  // test for recompiling the regular expression
+  {
+    Regex r;
+    REQUIRE(r.compile(R"(foo)") == true);
+    REQUIRE(r.exec("foo") == true);
+    REQUIRE(r.compile(R"(bar)") == true);
+    REQUIRE(r.exec("bar") == true);
+  }
 }