From 661911a3659e0640210e0f7fec398cae85fb18d0 Mon Sep 17 00:00:00 2001 From: Joshua Marantz Date: Thu, 15 Feb 2018 21:06:55 -0500 Subject: [PATCH 1/7] Find prefixes on ignored delimeters. Signed-off-by: Joshua Marantz --- source/common/stats/stats_impl.cc | 6 +++++- source/exe/main_common.cc | 1 + test/common/stats/stats_impl_test.cc | 1 + 3 files changed, 7 insertions(+), 1 deletion(-) diff --git a/source/common/stats/stats_impl.cc b/source/common/stats/stats_impl.cc index cb7055a5631c0..519641c3a202b 100644 --- a/source/common/stats/stats_impl.cc +++ b/source/common/stats/stats_impl.cc @@ -68,6 +68,10 @@ TagExtractorImpl::TagExtractorImpl(const std::string& name, const std::string& r : name_(name), prefix_(std::string(extractRegexPrefix(regex))), regex_(RegexUtil::parseRegex(regex)) {} +static bool regexStartsWithDot(absl::string_view regex) { + return absl::StartsWith(regex, "\\.") || absl::StartsWith(regex, "(?=\\.)"); +} + std::string TagExtractorImpl::extractRegexPrefix(absl::string_view regex) { std::string prefix; if (absl::StartsWith(regex, "^")) { @@ -75,7 +79,7 @@ std::string TagExtractorImpl::extractRegexPrefix(absl::string_view regex) { if (!absl::ascii_isalnum(regex[i]) && (regex[i] != '_')) { if (i > 1) { const bool last_char = i == regex.size() - 1; - if ((!last_char && (regex[i] == '\\') && (regex[i + 1] == '.')) || + if ((!last_char && regexStartsWithDot(regex.substr(i))) || (last_char && (regex[i] == '$'))) { prefix.append(regex.data() + 1, i - 1); } diff --git a/source/exe/main_common.cc b/source/exe/main_common.cc index c2436278e13f0..4f4dd04c96265 100644 --- a/source/exe/main_common.cc +++ b/source/exe/main_common.cc @@ -78,6 +78,7 @@ MainCommonBase::~MainCommonBase() { ares_library_cleanup(); } bool MainCommonBase::run() { switch (options_.mode()) { case Server::Mode::Serve: + exit(0); server_->run(); return true; case Server::Mode::Validate: { diff --git a/test/common/stats/stats_impl_test.cc b/test/common/stats/stats_impl_test.cc index 0f784dad7e161..0518fe31ae079 100644 --- a/test/common/stats/stats_impl_test.cc +++ b/test/common/stats/stats_impl_test.cc @@ -396,6 +396,7 @@ TEST(TagExtractorTest, ExtractRegexPrefix) { EXPECT_EQ("", extractRegexPrefix("^prefix(foo).")); EXPECT_EQ("prefix", extractRegexPrefix("^prefix\\.foo")); + EXPECT_EQ("prefix_optional", extractRegexPrefix("^prefix_optional(?=\\.)")); EXPECT_EQ("", extractRegexPrefix("^notACompleteToken")); // EXPECT_EQ("onlyToken", extractRegexPrefix("^onlyToken$")); // EXPECT_EQ("", extractRegexPrefix("(prefix)")); From c89ce642909c83d4b20b201cab6c01bf54453e9c Mon Sep 17 00:00:00 2001 From: Joshua Marantz Date: Thu, 15 Feb 2018 21:08:21 -0500 Subject: [PATCH 2/7] Revert "Find prefixes on ignored delimeters." This reverts commit 661911a3659e0640210e0f7fec398cae85fb18d0. Signed-off-by: Joshua Marantz --- source/common/stats/stats_impl.cc | 6 +----- source/exe/main_common.cc | 1 - test/common/stats/stats_impl_test.cc | 1 - 3 files changed, 1 insertion(+), 7 deletions(-) diff --git a/source/common/stats/stats_impl.cc b/source/common/stats/stats_impl.cc index 519641c3a202b..cb7055a5631c0 100644 --- a/source/common/stats/stats_impl.cc +++ b/source/common/stats/stats_impl.cc @@ -68,10 +68,6 @@ TagExtractorImpl::TagExtractorImpl(const std::string& name, const std::string& r : name_(name), prefix_(std::string(extractRegexPrefix(regex))), regex_(RegexUtil::parseRegex(regex)) {} -static bool regexStartsWithDot(absl::string_view regex) { - return absl::StartsWith(regex, "\\.") || absl::StartsWith(regex, "(?=\\.)"); -} - std::string TagExtractorImpl::extractRegexPrefix(absl::string_view regex) { std::string prefix; if (absl::StartsWith(regex, "^")) { @@ -79,7 +75,7 @@ std::string TagExtractorImpl::extractRegexPrefix(absl::string_view regex) { if (!absl::ascii_isalnum(regex[i]) && (regex[i] != '_')) { if (i > 1) { const bool last_char = i == regex.size() - 1; - if ((!last_char && regexStartsWithDot(regex.substr(i))) || + if ((!last_char && (regex[i] == '\\') && (regex[i + 1] == '.')) || (last_char && (regex[i] == '$'))) { prefix.append(regex.data() + 1, i - 1); } diff --git a/source/exe/main_common.cc b/source/exe/main_common.cc index 4f4dd04c96265..c2436278e13f0 100644 --- a/source/exe/main_common.cc +++ b/source/exe/main_common.cc @@ -78,7 +78,6 @@ MainCommonBase::~MainCommonBase() { ares_library_cleanup(); } bool MainCommonBase::run() { switch (options_.mode()) { case Server::Mode::Serve: - exit(0); server_->run(); return true; case Server::Mode::Validate: { diff --git a/test/common/stats/stats_impl_test.cc b/test/common/stats/stats_impl_test.cc index 0518fe31ae079..0f784dad7e161 100644 --- a/test/common/stats/stats_impl_test.cc +++ b/test/common/stats/stats_impl_test.cc @@ -396,7 +396,6 @@ TEST(TagExtractorTest, ExtractRegexPrefix) { EXPECT_EQ("", extractRegexPrefix("^prefix(foo).")); EXPECT_EQ("prefix", extractRegexPrefix("^prefix\\.foo")); - EXPECT_EQ("prefix_optional", extractRegexPrefix("^prefix_optional(?=\\.)")); EXPECT_EQ("", extractRegexPrefix("^notACompleteToken")); // EXPECT_EQ("onlyToken", extractRegexPrefix("^onlyToken$")); // EXPECT_EQ("", extractRegexPrefix("(prefix)")); From f6c0f47abe4d83d690e3da329048a7023a44a15d Mon Sep 17 00:00:00 2001 From: Joshua Marantz Date: Thu, 15 Feb 2018 21:12:18 -0500 Subject: [PATCH 3/7] Find prefixes on ignored delimeters. Signed-off-by: Joshua Marantz --- source/common/stats/stats_impl.cc | 6 +++++- source/exe/main_common.cc | 1 + test/common/stats/stats_impl_test.cc | 1 + 3 files changed, 7 insertions(+), 1 deletion(-) diff --git a/source/common/stats/stats_impl.cc b/source/common/stats/stats_impl.cc index cb7055a5631c0..519641c3a202b 100644 --- a/source/common/stats/stats_impl.cc +++ b/source/common/stats/stats_impl.cc @@ -68,6 +68,10 @@ TagExtractorImpl::TagExtractorImpl(const std::string& name, const std::string& r : name_(name), prefix_(std::string(extractRegexPrefix(regex))), regex_(RegexUtil::parseRegex(regex)) {} +static bool regexStartsWithDot(absl::string_view regex) { + return absl::StartsWith(regex, "\\.") || absl::StartsWith(regex, "(?=\\.)"); +} + std::string TagExtractorImpl::extractRegexPrefix(absl::string_view regex) { std::string prefix; if (absl::StartsWith(regex, "^")) { @@ -75,7 +79,7 @@ std::string TagExtractorImpl::extractRegexPrefix(absl::string_view regex) { if (!absl::ascii_isalnum(regex[i]) && (regex[i] != '_')) { if (i > 1) { const bool last_char = i == regex.size() - 1; - if ((!last_char && (regex[i] == '\\') && (regex[i + 1] == '.')) || + if ((!last_char && regexStartsWithDot(regex.substr(i))) || (last_char && (regex[i] == '$'))) { prefix.append(regex.data() + 1, i - 1); } diff --git a/source/exe/main_common.cc b/source/exe/main_common.cc index c2436278e13f0..4f4dd04c96265 100644 --- a/source/exe/main_common.cc +++ b/source/exe/main_common.cc @@ -78,6 +78,7 @@ MainCommonBase::~MainCommonBase() { ares_library_cleanup(); } bool MainCommonBase::run() { switch (options_.mode()) { case Server::Mode::Serve: + exit(0); server_->run(); return true; case Server::Mode::Validate: { diff --git a/test/common/stats/stats_impl_test.cc b/test/common/stats/stats_impl_test.cc index 0f784dad7e161..0518fe31ae079 100644 --- a/test/common/stats/stats_impl_test.cc +++ b/test/common/stats/stats_impl_test.cc @@ -396,6 +396,7 @@ TEST(TagExtractorTest, ExtractRegexPrefix) { EXPECT_EQ("", extractRegexPrefix("^prefix(foo).")); EXPECT_EQ("prefix", extractRegexPrefix("^prefix\\.foo")); + EXPECT_EQ("prefix_optional", extractRegexPrefix("^prefix_optional(?=\\.)")); EXPECT_EQ("", extractRegexPrefix("^notACompleteToken")); // EXPECT_EQ("onlyToken", extractRegexPrefix("^onlyToken$")); // EXPECT_EQ("", extractRegexPrefix("(prefix)")); From 379bc3bb0c2b805e384a3239d4c0c766921cc9d7 Mon Sep 17 00:00:00 2001 From: Joshua Marantz Date: Thu, 15 Feb 2018 22:25:33 -0500 Subject: [PATCH 4/7] remove stray debugging exit(0) Signed-off-by: Joshua Marantz --- source/exe/main_common.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/source/exe/main_common.cc b/source/exe/main_common.cc index 4f4dd04c96265..c2436278e13f0 100644 --- a/source/exe/main_common.cc +++ b/source/exe/main_common.cc @@ -78,7 +78,6 @@ MainCommonBase::~MainCommonBase() { ares_library_cleanup(); } bool MainCommonBase::run() { switch (options_.mode()) { case Server::Mode::Serve: - exit(0); server_->run(); return true; case Server::Mode::Validate: { From fa9acc6bf23e74312ab37ab65b2fb8d16664cb93 Mon Sep 17 00:00:00 2001 From: Joshua Marantz Date: Mon, 26 Feb 2018 13:06:50 -0500 Subject: [PATCH 5/7] Move static function to anon namespace, and update style guide to require that generally. Signed-off-by: Joshua Marantz --- STYLE.md | 6 +++++- source/common/stats/stats_impl.cc | 8 ++++---- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/STYLE.md b/STYLE.md index b5767ae812949..8df0c6a093f16 100644 --- a/STYLE.md +++ b/STYLE.md @@ -67,7 +67,11 @@ annotations](https://github.com/abseil/abseil-cpp/blob/master/absl/base/thread_annotations.h), such as `GUARDED_BY`, should be used for shared state guarded by locks/mutexes. - +* Functions intended to be local to a cc file should be declared in an anonymonus namespace, + rather than using the 'static' keyword. Note that the + [Google styleguide](https://google.github.io/styleguide/cppguide.html#Unnamed_Namespaces_and_Static_Variables) + allows either, but in Envoy we prefer annonymous namespaces. + # Error handling A few general notes on our error handling philosophy: diff --git a/source/common/stats/stats_impl.cc b/source/common/stats/stats_impl.cc index 519641c3a202b..3b81301f120a0 100644 --- a/source/common/stats/stats_impl.cc +++ b/source/common/stats/stats_impl.cc @@ -28,6 +28,10 @@ size_t roundUpMultipleNaturalAlignment(size_t val) { return (val + multiple - 1) & ~(multiple - 1); } +bool regexStartsWithDot(absl::string_view regex) { + return absl::StartsWith(regex, "\\.") || absl::StartsWith(regex, "(?=\\.)"); +} + } // namespace size_t RawStatData::size() { @@ -68,10 +72,6 @@ TagExtractorImpl::TagExtractorImpl(const std::string& name, const std::string& r : name_(name), prefix_(std::string(extractRegexPrefix(regex))), regex_(RegexUtil::parseRegex(regex)) {} -static bool regexStartsWithDot(absl::string_view regex) { - return absl::StartsWith(regex, "\\.") || absl::StartsWith(regex, "(?=\\.)"); -} - std::string TagExtractorImpl::extractRegexPrefix(absl::string_view regex) { std::string prefix; if (absl::StartsWith(regex, "^")) { From e4a2b362375a5e66f9ab0ed8469c03a4dc7638e0 Mon Sep 17 00:00:00 2001 From: Joshua Marantz Date: Mon, 26 Feb 2018 13:08:24 -0500 Subject: [PATCH 6/7] format fix. Signed-off-by: Joshua Marantz --- STYLE.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/STYLE.md b/STYLE.md index 8df0c6a093f16..9c12df604d3c0 100644 --- a/STYLE.md +++ b/STYLE.md @@ -68,10 +68,10 @@ such as `GUARDED_BY`, should be used for shared state guarded by locks/mutexes. * Functions intended to be local to a cc file should be declared in an anonymonus namespace, - rather than using the 'static' keyword. Note that the + rather than using the 'static' keyword. Note that the [Google styleguide](https://google.github.io/styleguide/cppguide.html#Unnamed_Namespaces_and_Static_Variables) allows either, but in Envoy we prefer annonymous namespaces. - + # Error handling A few general notes on our error handling philosophy: From 7a63a77d55088c9c50e1fef6c262edd8ba1b354d Mon Sep 17 00:00:00 2001 From: Joshua Marantz Date: Mon, 26 Feb 2018 14:52:53 -0500 Subject: [PATCH 7/7] Spell "Google C++ style guide" consistently with elsewhere in this doc. Signed-off-by: Joshua Marantz --- STYLE.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/STYLE.md b/STYLE.md index 9c12df604d3c0..c6e203f8f9fe8 100644 --- a/STYLE.md +++ b/STYLE.md @@ -69,7 +69,7 @@ locks/mutexes. * Functions intended to be local to a cc file should be declared in an anonymonus namespace, rather than using the 'static' keyword. Note that the - [Google styleguide](https://google.github.io/styleguide/cppguide.html#Unnamed_Namespaces_and_Static_Variables) + [Google C++ style guide](https://google.github.io/styleguide/cppguide.html#Unnamed_Namespaces_and_Static_Variables) allows either, but in Envoy we prefer annonymous namespaces. # Error handling