You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by "ywkaras (via GitHub)" <gi...@apache.org> on 2024/04/12 01:12:11 UTC
[PR] Avoid including pcre2.h in Regex.h. [trafficserver]
ywkaras opened a new pull request, #11246:
URL: https://github.com/apache/trafficserver/pull/11246
(no comment)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Avoid including pcre2.h in Regex.h. [trafficserver]
Posted by "bneradt (via GitHub)" <gi...@apache.org>.
bneradt commented on code in PR #11246:
URL: https://github.com/apache/trafficserver/pull/11246#discussion_r1562632895
##########
include/tsutil/Regex.h:
##########
@@ -28,14 +28,11 @@
#include <vector>
#include <memory>
-#define PCRE2_CODE_UNIT_WIDTH 8
-#include <pcre2.h>
-
/// @brief Match flags for regular expression evaluation.
enum REFlags {
- RE_CASE_INSENSITIVE = PCRE2_CASELESS, ///< Ignore case (default: case sensitive).
- RE_UNANCHORED = PCRE2_MULTILINE, ///< Unanchored (DFA defaults to anchored).
- RE_ANCHORED = PCRE2_ANCHORED, ///< Anchored (Regex defaults to unanchored).
+ RE_CASE_INSENSITIVE = 0x00000008u, ///< Ignore case (default: case sensitive).
+ RE_UNANCHORED = 0x00000400u, ///< Unanchored (DFA defaults to anchored).
+ RE_ANCHORED = 0x80000000u, ///< Anchored (Regex defaults to unanchored).
Review Comment:
These values seem pretty magical now without the PCRE2 names. Can you add a comment above the enum saying these are taken directly from pcre2.h but without the include to avoid the header.
##########
include/tsutil/Regex.h:
##########
@@ -63,19 +60,23 @@ class RegexMatches
size_t *get_ovector_pointer();
int32_t size() const;
-protected:
- pcre2_match_data *get_match_data();
- void set_subject(std::string_view subject);
- void set_size(int32_t size);
-
private:
constexpr static uint32_t DEFAULT_MATCHES = 10;
static void *malloc(size_t size, void *caller);
- pcre2_match_data *_match_data = nullptr;
std::string_view _subject;
char _buffer[24 + 96 + 16 * DEFAULT_MATCHES]; // 24 bytes for the general context, 96 bytes overhead, 16 bytes per match.
size_t _buffer_bytes_used = 0;
int32_t _size = 0;
+
+ struct _MatchData;
+ class _MatchDataPtr
+ {
+ friend struct _MatchData;
+
+ private:
+ void *_ptr = nullptr;
+ };
+ _MatchDataPtr _match_data;
Review Comment:
A comment here would be helpful to future readers. Let's explain that this effectively wraps a void* so that we can avoid requiring the pcre2.h include for the user of the Regex API. (If I'm understanding it correctly.)
##########
src/tsutil/Regex.cc:
##########
@@ -23,11 +23,18 @@
#include "tsutil/Regex.h"
+#define PCRE2_CODE_UNIT_WIDTH 8
+#include <pcre2.h>
+
#include <array>
#include <assert.h>
#include <vector>
#include <mutex>
+static_assert(RE_CASE_INSENSITIVE == PCRE2_CASELESS, "Update RE_CASE_INSERSITIVE for current PCRE2 version.");
+static_assert(RE_UNANCHORED == PCRE2_MULTILINE, "Update RE_MULTILINE for current PCRE2 version.");
+static_assert(RE_ANCHORED == PCRE2_ANCHORED, "Update RE_ANCHORED for current PCRE2 version.");
Review Comment:
Nice.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Avoid including pcre2.h in Regex.h. [trafficserver]
Posted by "cmcfarlen (via GitHub)" <gi...@apache.org>.
cmcfarlen commented on PR #11246:
URL: https://github.com/apache/trafficserver/pull/11246#issuecomment-2057095118
Cherry-picked to v10.0.x
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Avoid including pcre2.h in Regex.h. [trafficserver]
Posted by "ywkaras (via GitHub)" <gi...@apache.org>.
ywkaras merged PR #11246:
URL: https://github.com/apache/trafficserver/pull/11246
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Avoid including pcre2.h in Regex.h. [trafficserver]
Posted by "ywkaras (via GitHub)" <gi...@apache.org>.
ywkaras commented on PR #11246:
URL: https://github.com/apache/trafficserver/pull/11246#issuecomment-2051770609
[approve ci autest]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Avoid including pcre2.h in Regex.h. [trafficserver]
Posted by "brbzull0 (via GitHub)" <gi...@apache.org>.
brbzull0 commented on PR #11246:
URL: https://github.com/apache/trafficserver/pull/11246#issuecomment-2051383527
[approve ci autest]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org