Skip to content

Change Regex class to use PCRE2 - #11014

Merged
bryancall merged 12 commits into
apache:masterfrom
bryancall:regex_interface
Feb 29, 2024
Merged

Change Regex class to use PCRE2#11014
bryancall merged 12 commits into
apache:masterfrom
bryancall:regex_interface

Conversation

@bryancall

@bryancall bryancall commented Jan 26, 2024

Copy link
Copy Markdown
Contributor

Converted Regex class from PCRE to PCRE2

@bryancall bryancall added this to the 10.0.0 milestone Jan 26, 2024
@bryancall bryancall self-assigned this Jan 26, 2024
@bryancall
bryancall requested a review from cmcfarlen January 29, 2024 23:24
@bryancall
bryancall marked this pull request as draft February 8, 2024 22:13
@bryancall bryancall changed the title Change the Regex interface to better support use cases in ATS Change Regex class to use PCRE2 Feb 8, 2024
@bryancall
bryancall marked this pull request as ready for review February 15, 2024 21:01
@bryancall
bryancall requested a review from traeak February 26, 2024 23:20

@traeak traeak left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Try this patch (inspired by cripts):

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 920d15835..10913055e 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -258,7 +258,8 @@ 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)

Comment thread include/tsutil/Regex.h Outdated
Comment thread src/tsutil/Regex.cc Outdated
@bryancall

bryancall commented Feb 28, 2024

Copy link
Copy Markdown
Contributor Author

Valgrind test

14:13:05 zeus:(regex_interface)~/dev/apache/trafficserver/build$ ninja && valgrind ./src/tsutil/test_tsutil
[0/2] Re-checking globbed directories...
[152/152] Linking CXX executable src/traffic_server/traffic_server
==1309146== Memcheck, a memory error detector
==1309146== Copyright (C) 2002-2022, and GNU GPL'd, by Julian Seward et al.
==1309146== Using Valgrind-3.22.0 and LibVEX; rerun with -h for copyright info
==1309146== Command: ./src/tsutil/test_tsutil
==1309146==
===============================================================================
All tests passed (182 assertions in 8 test cases)

==1309146==
==1309146== HEAP SUMMARY:
==1309146==     in use at exit: 288 bytes in 6 blocks
==1309146==   total heap usage: 2,955 allocs, 2,949 frees, 684,530 bytes allocated
==1309146==
==1309146== LEAK SUMMARY:
==1309146==    definitely lost: 0 bytes in 0 blocks
==1309146==    indirectly lost: 0 bytes in 0 blocks
==1309146==      possibly lost: 0 bytes in 0 blocks
==1309146==    still reachable: 288 bytes in 6 blocks
==1309146==         suppressed: 0 bytes in 0 blocks
==1309146== Rerun with --leak-check=full to see details of leaked memory
==1309146==
==1309146== For lists of detected and suppressed errors, rerun with: -s
==1309146== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

@bryancall

Copy link
Copy Markdown
Contributor Author

Rebased the PR. There was a change in tls_bridge/CMakeLists.txt that was conflicting.

@traeak traeak left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes look fine.

@bryancall
bryancall merged commit 8e1c1b9 into apache:master Feb 29, 2024
@cmcfarlen

Copy link
Copy Markdown
Contributor

Cherry-picked to v10.0.x

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants