This is an automated email from the ASF dual-hosted git repository. cmcfarlen pushed a commit to branch 10.0.x in repository https://gitbox.apache.org/repos/asf/trafficserver.git
commit 2a9c887eedae749b7c095d21bfc5b98e396e6544 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 52613f017b..06e8bb1d84 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()
