From 4580584b11a8952b585875ff8cec32785592d780 Mon Sep 17 00:00:00 2001 From: Stephane Janel Date: Sat, 21 Oct 2023 12:35:06 +0200 Subject: [PATCH] Extract gethostname into a dedicated module. Add unit test and better error management. --- src/engine/src/coincentercommands.cpp | 5 +- .../src/prometheusmetricgateway.cpp | 27 +++------ src/objects/include/logginginfo.hpp | 2 +- src/tech/CMakeLists.txt | 8 +++ src/tech/include/gethostname.hpp | 8 +++ src/tech/src/gethostname.cpp | 56 +++++++++++++++++++ src/tech/test/gethostname_test.cpp | 12 ++++ 7 files changed, 96 insertions(+), 22 deletions(-) create mode 100644 src/tech/include/gethostname.hpp create mode 100644 src/tech/src/gethostname.cpp create mode 100644 src/tech/test/gethostname_test.cpp diff --git a/src/engine/src/coincentercommands.cpp b/src/engine/src/coincentercommands.cpp index c9abd070..89bc6981 100644 --- a/src/engine/src/coincentercommands.cpp +++ b/src/engine/src/coincentercommands.cpp @@ -4,12 +4,12 @@ #include #include #include +#include #include #include "cct_invalid_argument_exception.hpp" -#include "cct_log.hpp" -#include "cct_string.hpp" #include "cct_vector.hpp" +#include "coincentercommand.hpp" #include "coincentercommandtype.hpp" #include "coincenteroptions.hpp" #include "commandlineoptionsparser.hpp" @@ -19,6 +19,7 @@ #include "monetaryamount.hpp" #include "ordersconstraints.hpp" #include "priceoptions.hpp" +#include "priceoptionsdef.hpp" #include "stringoptionparser.hpp" #include "timedef.hpp" #include "tradedefinitions.hpp" diff --git a/src/monitoring/src/prometheusmetricgateway.cpp b/src/monitoring/src/prometheusmetricgateway.cpp index 08fcefa5..a0ef2438 100644 --- a/src/monitoring/src/prometheusmetricgateway.cpp +++ b/src/monitoring/src/prometheusmetricgateway.cpp @@ -7,6 +7,7 @@ #include #include +#include #include #include #include @@ -18,39 +19,27 @@ #include "abstractmetricgateway.hpp" #include "cct_exception.hpp" #include "cct_log.hpp" +#include "gethostname.hpp" #include "metric.hpp" #include "monitoringinfo.hpp" #include "timedef.hpp" -#ifdef CCT_MSVC -#include -#else -#include -#endif - namespace cct { namespace { -constexpr int kHTTPSuccessReturnCode = 200; +constexpr auto kHTTPSuccessReturnCode = 200; // Constants to control frequency of flushes to Prometheus instance -constexpr Duration kPrometheusAutoFlushPeriod = std::chrono::minutes(3); -constexpr int kCheckFlushCounter = 20; - -std::string GetHostName() { - char hostname[1024]; - if (::gethostname(hostname, sizeof(hostname)) != 0) { - hostname[0] = '\0'; - } - return hostname; -} +constexpr auto kPrometheusAutoFlushPeriod = std::chrono::minutes(3); +constexpr auto kCheckFlushCounter = 20; } // namespace PrometheusMetricGateway::PrometheusMetricGateway(const MonitoringInfo& monitoringInfo) : AbstractMetricGateway(monitoringInfo), _gateway(std::string(monitoringInfo.address()), std::to_string(monitoringInfo.port()), - std::string(monitoringInfo.jobName()), prometheus::Gateway::GetInstanceLabel(GetHostName()), + std::string(monitoringInfo.jobName()), + prometheus::Gateway::GetInstanceLabel(GetHostName().toStdString()), std::string(monitoringInfo.username()), std::string(monitoringInfo.password())), _registry(std::make_shared()), _lastFlushedTime(Clock::now()), @@ -63,7 +52,7 @@ PrometheusMetricGateway::~PrometheusMetricGateway() { // We should not throw in a destructor - catch any exception and do nothing, not even a log (it could throw) try { flush(); - } catch (...) { + } catch (const std::exception&) { } } diff --git a/src/objects/include/logginginfo.hpp b/src/objects/include/logginginfo.hpp index 7522f622..d4ba3b2f 100644 --- a/src/objects/include/logginginfo.hpp +++ b/src/objects/include/logginginfo.hpp @@ -16,7 +16,7 @@ namespace cct { /// @brief Encapsulates loggers lifetime and set-up. class LoggingInfo { public: - static constexpr int64_t kDefaultFileSizeInBytes = 5 * 1024 * 1024; + static constexpr int64_t kDefaultFileSizeInBytes = 5L * 1024 * 1024; static constexpr int32_t kDefaultNbMaxFiles = 10; static constexpr char const *const kOutputLoggerName = "output"; diff --git a/src/tech/CMakeLists.txt b/src/tech/CMakeLists.txt index 264771fe..414a06e1 100644 --- a/src/tech/CMakeLists.txt +++ b/src/tech/CMakeLists.txt @@ -62,6 +62,14 @@ add_unit_test( nlohmann_json::nlohmann_json ) +add_unit_test( + gethostname_test + src/gethostname.cpp + test/gethostname_test.cpp + DEFINITIONS + CCT_DISABLE_SPDLOG +) + add_unit_test( mathhelpers_test test/mathhelpers_test.cpp diff --git a/src/tech/include/gethostname.hpp b/src/tech/include/gethostname.hpp new file mode 100644 index 00000000..6ff33c5f --- /dev/null +++ b/src/tech/include/gethostname.hpp @@ -0,0 +1,8 @@ +#pragma once + +#include "cct_string.hpp" + +namespace cct { +/// Safe version of gethostname, working in POSIX and Windows with similar behavior. +string GetHostName(); +} // namespace cct \ No newline at end of file diff --git a/src/tech/src/gethostname.cpp b/src/tech/src/gethostname.cpp new file mode 100644 index 00000000..c19bc7d8 --- /dev/null +++ b/src/tech/src/gethostname.cpp @@ -0,0 +1,56 @@ +#include "gethostname.hpp" + +#include "cct_exception.hpp" +#include "cct_string.hpp" +#ifdef CCT_MSVC +#include +#else +#include +#include +#endif + +namespace cct { +/// Safe version of gethostname, working in POSIX and Windows with similar behavior. +string GetHostName() { + string hostname(16U, '\0'); + static constexpr std::size_t kMaxHostNameSize = 1024; + std::size_t nullTerminatedCharPos = 0; + do { + int errorCode = ::gethostname(hostname.data(), hostname.size() - 1); + if (errorCode != 0) { +#ifdef CCT_MSVC + // In Windows, too small buffer returns an error WSAEFAULT + if (errorCode == WSAEFAULT) { +#else + // In Posix, too small buffer returns an error ENAMETOOLONG set in errno + if (errno == ENAMETOOLONG) { +#endif + nullTerminatedCharPos = hostname.size() - 1U; + hostname.resize(2 * hostname.size(), '\0'); + continue; + } + throw exception("Error {} in gethostname", errorCode); + } +#ifndef CCT_MSVC + if (hostname.back() != '\0') { + // In POSIX, if the null-terminated hostname is too large to fit, then the name is truncated, and no error is + // returned, meaning that last char has necessarily been written to + nullTerminatedCharPos = hostname.size() - 1U; + hostname.resize(2 * hostname.size(), '\0'); + continue; + } +#endif + if (hostname.size() > kMaxHostNameSize) { + throw exception("Unexpected host name size length {}", hostname.size()); + } + break; + } while (true); + + auto hostnameSize = hostname.find('\0', nullTerminatedCharPos); + if (hostnameSize == string::npos) { + throw exception("Unexpected error in GetHostName algorithm"); + } + hostname.resize(hostnameSize); + return hostname; +} +} // namespace cct \ No newline at end of file diff --git a/src/tech/test/gethostname_test.cpp b/src/tech/test/gethostname_test.cpp new file mode 100644 index 00000000..b6f42b8c --- /dev/null +++ b/src/tech/test/gethostname_test.cpp @@ -0,0 +1,12 @@ +#include "gethostname.hpp" + +#include + +namespace cct { +TEST(GetHostNameTest, Default) { + auto currentHostName = GetHostName(); + + EXPECT_FALSE(currentHostName.empty()); + EXPECT_EQ(currentHostName.find('\0'), string::npos); +} +} // namespace cct \ No newline at end of file