This is an automated email from the ASF dual-hosted git repository.
zwoop pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
The following commit(s) were added to refs/heads/master by this push:
new 0a53513961 Fixes Bundles errors, and better checks (#11398)
0a53513961 is described below
commit 0a53513961cbe55a6bfde32be208c0e04a70b0f1
Author: Leif Hedstrom <[email protected]>
AuthorDate: Tue Jun 4 13:05:06 2024 -0600
Fixes Bundles errors, and better checks (#11398)
---
include/cripts/Bundle.hpp | 11 ++---------
include/cripts/Bundles/Common.hpp | 15 +++++++++++----
include/cripts/Bundles/LogsMetrics.hpp | 19 ++++++++++++-------
include/cripts/Epilogue.hpp | 2 +-
include/cripts/Headers.hpp | 4 +---
include/cripts/Instance.hpp | 12 ++++++++++++
src/cripts/Bundles/Common.cc | 4 ++--
src/cripts/Bundles/LogsMetrics.cc | 2 +-
src/cripts/CMakeLists.txt | 9 +++++++--
9 files changed, 49 insertions(+), 29 deletions(-)
diff --git a/include/cripts/Bundle.hpp b/include/cripts/Bundle.hpp
index 4551b519e7..955ec9b88f 100644
--- a/include/cripts/Bundle.hpp
+++ b/include/cripts/Bundle.hpp
@@ -72,6 +72,8 @@ namespace Bundle
void operator=(const self_type &) = delete;
virtual ~Base() = default;
+ virtual const Cript::string &name() const = 0;
+
void
needCallback(Cript::Callbacks cb)
{
@@ -141,17 +143,8 @@ namespace Bundle
protected:
unsigned _callbacks = 0;
-
}; // Class Base
- // These are used in the preamble, to see if a function is implemented in a
bundle.
- template <typename T>
- bool
- checkDoRemap()
- {
- return (std::is_same_v<decltype(&T::doRemap), void (T::*)()>);
- }
-
} // namespace Bundle
} // namespace Cript
diff --git a/include/cripts/Bundles/Common.hpp
b/include/cripts/Bundles/Common.hpp
index 6fe8933404..9bf07c898c 100644
--- a/include/cripts/Bundles/Common.hpp
+++ b/include/cripts/Bundles/Common.hpp
@@ -42,11 +42,17 @@ public:
{
auto *entry = new self_type();
- inst.bundles.push_back(entry);
+ inst.addBundle(entry);
return *entry;
}
+ const Cript::string &
+ name() const override
+ {
+ return _name;
+ }
+
self_type &
dscp(int val)
{
@@ -70,9 +76,10 @@ public:
void doRemap(Cript::Context *context) override;
private:
- Cript::string _cc = "";
- int _dscp = 0;
- bool _force_cc = false;
+ static const Cript::string _name;
+ Cript::string _cc = "";
+ int _dscp = 0;
+ bool _force_cc = false;
};
} // namespace Bundle
diff --git a/include/cripts/Bundles/LogsMetrics.hpp
b/include/cripts/Bundles/LogsMetrics.hpp
index 6d4d80bcd3..e5e8c8ad8b 100644
--- a/include/cripts/Bundles/LogsMetrics.hpp
+++ b/include/cripts/Bundles/LogsMetrics.hpp
@@ -34,8 +34,6 @@ class LogsMetrics : public Cript::Bundle::Base
using self_type = LogsMetrics;
public:
- using super_type::Base;
-
LogsMetrics(Cript::Instance *inst) : _inst(inst) {}
static self_type &
@@ -43,11 +41,17 @@ public:
{
auto *entry = new self_type(&inst);
- inst.bundles.push_back(entry);
+ inst.addBundle(entry);
return *entry;
}
+ const Cript::string &
+ name() const override
+ {
+ return _name;
+ }
+
self_type &propstats(const Cript::string_view &label); // In LogsMetrics.cc
self_type &
@@ -76,10 +80,11 @@ public:
void doRemap(Cript::Context *context) override;
private:
- Cript::Instance *_inst; // This Bundle needs the instance for
access to the instance metrics
- Cript::string _label = ""; // Propstats label
- int _log_sample = 0; // Log sampling
- bool _tcpinfo = false; // Turn on TCP info logging
+ static const Cript::string _name;
+ Cript::Instance *_inst; // This Bundle needs the
instance for access to the instance metrics
+ Cript::string _label = ""; // Propstats label
+ int _log_sample = 0; // Log sampling
+ bool _tcpinfo = false; // Turn on TCP info logging
};
} // namespace Bundle
diff --git a/include/cripts/Epilogue.hpp b/include/cripts/Epilogue.hpp
index 0a710a5b99..2f02c2d845 100644
--- a/include/cripts/Epilogue.hpp
+++ b/include/cripts/Epilogue.hpp
@@ -408,7 +408,7 @@ TSRemapNewInstance(int argc, char *argv[], void **ih, char
* /* errbuf ATS_UNUSE
for (auto &bundle : inst->bundles) {
// Collect all the callbacks needed from all bundles, and validate them
- if (bundle->validate(errors)) {
+ if (!bundle->validate(errors)) {
inst->needCallback(bundle->callbacks());
}
}
diff --git a/include/cripts/Headers.hpp b/include/cripts/Headers.hpp
index c36e9ecc62..55ab969863 100644
--- a/include/cripts/Headers.hpp
+++ b/include/cripts/Headers.hpp
@@ -405,9 +405,7 @@ public:
void
erase(const Cript::string_view header)
{
- auto p = operator[](header);
-
- p.clear();
+ operator[](header) = "";
}
Iterator begin();
diff --git a/include/cripts/Instance.hpp b/include/cripts/Instance.hpp
index 643e71fc7d..7b6cf13c53 100644
--- a/include/cripts/Instance.hpp
+++ b/include/cripts/Instance.hpp
@@ -58,6 +58,18 @@ public:
bool deletePlugin(const Cript::string &tag);
void initialize(int argc, char *argv[], const char *filename);
+ void
+ addBundle(Cript::Bundle::Base *bundle)
+ {
+ for (auto &it : bundles) {
+ if (it->name() == bundle->name()) {
+ TSReleaseAssert(!"Duplicate bundle");
+ }
+ }
+
+ bundles.push_back(bundle);
+ }
+
// This allows Bundles to require hooks as well.
void
needCallback(Cript::Callbacks cb)
diff --git a/src/cripts/Bundles/Common.cc b/src/cripts/Bundles/Common.cc
index 7c1dbb41fa..495efaf480 100644
--- a/src/cripts/Bundles/Common.cc
+++ b/src/cripts/Bundles/Common.cc
@@ -22,8 +22,8 @@
namespace Bundle
{
+const Cript::string Common::_name = "Bundle::Common";
-// Bundle::Common
bool
Common::validate(std::vector<Cript::Bundle::Error> &errors) const
{
@@ -31,7 +31,7 @@ Common::validate(std::vector<Cript::Bundle::Error> &errors)
const
// The .dscp() can only be 0 - 63
if (_dscp < 0 || _dscp > 63) {
- errors.emplace_back("dscp must be between 0 and 63", "Common", "dscp");
+ errors.emplace_back("dscp must be between 0 and 63", name(), "dscp");
failed = true;
}
diff --git a/src/cripts/Bundles/LogsMetrics.cc
b/src/cripts/Bundles/LogsMetrics.cc
index 36b04465a9..1a40319312 100644
--- a/src/cripts/Bundles/LogsMetrics.cc
+++ b/src/cripts/Bundles/LogsMetrics.cc
@@ -22,8 +22,8 @@
namespace Bundle
{
+const Cript::string LogsMetrics::_name = "Bundle::LogsMetrics";
-// Bundle::LogsMetrics
namespace
{
enum {
diff --git a/src/cripts/CMakeLists.txt b/src/cripts/CMakeLists.txt
index 31a673dfc8..955cf5c836 100644
--- a/src/cripts/CMakeLists.txt
+++ b/src/cripts/CMakeLists.txt
@@ -45,8 +45,10 @@ set(CRIPTS_PUBLIC_HEADERS
${PROJECT_SOURCE_DIR}/include/cripts/Urls.hpp
${PROJECT_SOURCE_DIR}/include/cripts/UUID.hpp
${PROJECT_SOURCE_DIR}/include/cripts/Configs.hpp
- ${PROJECT_SOURCE_DIR}/include/cripts/Bundles/Common.hpp
- ${PROJECT_SOURCE_DIR}/include/cripts/Bundles/LogsMetrics.hpp
+)
+
+set(CRIPTS_BUNDLE_HEADERS
${PROJECT_SOURCE_DIR}/include/cripts/Bundles/Common.hpp
+
${PROJECT_SOURCE_DIR}/include/cripts/Bundles/LogsMetrics.hpp
)
add_library(cripts SHARED ${CPP_FILES})
@@ -64,6 +66,9 @@ set_target_properties(
install(TARGETS cripts PUBLIC_HEADER DESTINATION
${CMAKE_INSTALL_INCLUDEDIR}/cripts)
+# The PUBLIC_HEADER target can't handle include files in a subdirectory, so do
those manually.
+install(FILES ${CRIPTS_BUNDLE_HEADERS} DESTINATION
${CMAKE_INSTALL_INCLUDEDIR}/cripts/Bundles)
+
if(APPLE)
target_link_options(cripts PRIVATE -undefined dynamic_lookup)
endif()
