From b13d9f290676413c190ba987c57811e3d1991522 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 | 23 ++++++ src/tech/include/gethostname.hpp | 30 ++++++++ src/tech/src/gethostname.cpp | 74 +++++++++++++++++++ src/tech/test/gethostname_test.cpp | 13 ++++ 7 files changed, 152 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..a3c211f6 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(HostNameGetter().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..814aec3b 100644 --- a/src/tech/CMakeLists.txt +++ b/src/tech/CMakeLists.txt @@ -62,6 +62,29 @@ add_unit_test( nlohmann_json::nlohmann_json ) + + +if(WIN32) + add_unit_test( + gethostname_test + src/gethostname.cpp + test/gethostname_test.cpp + DEFINITIONS + CCT_DISABLE_SPDLOG + LIBRARIES + wsock32 + ws2_32 + ) +else() + add_unit_test( + gethostname_test + src/gethostname.cpp + test/gethostname_test.cpp + DEFINITIONS + CCT_DISABLE_SPDLOG + ) +endif() + 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..7b97f26f --- /dev/null +++ b/src/tech/include/gethostname.hpp @@ -0,0 +1,30 @@ +#pragma once + +#include "cct_config.hpp" +#include "cct_string.hpp" + +namespace cct { +class HostNameGetter { + public: +#ifdef CCT_MSVC + HostNameGetter(); +#else + HostNameGetter() noexcept = default; +#endif + + HostNameGetter(const HostNameGetter &) = delete; + HostNameGetter &operator=(const HostNameGetter &) = delete; + HostNameGetter(HostNameGetter &&) = delete; + HostNameGetter &operator=(HostNameGetter &&) = delete; + +#ifdef CCT_MSVC + ~HostNameGetter(); +#else + ~HostNameGetter() = default; +#endif + + /// Safe version of gethostname, working in POSIX and Windows with similar behavior. + string getHostName() const; +}; + +} // 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..5e5120d9 --- /dev/null +++ b/src/tech/src/gethostname.cpp @@ -0,0 +1,74 @@ +#include "gethostname.hpp" + +#include + +#include "cct_config.hpp" +#include "cct_exception.hpp" +#include "cct_string.hpp" +#ifdef CCT_MSVC +#include +#include +#else +#include +#include +#endif + +namespace cct { +#ifdef CCT_MSVC +HostNameGetter::HostNameGetter() { + WSADATA wsaData; + WORD wVersionRequested = MAKEWORD(2, 2); + auto errCode = WSAStartup(wVersionRequested, &wsaData); + if (errCode != 0) { + throw exception("Error {} in WSAStartup", errCode); + } +} + +HostNameGetter::~HostNameGetter() { WSACleanup(); } +#endif + +string HostNameGetter::getHostName() const { + string hostname(4U, '\0'); + static constexpr std::size_t kMaxHostNameSize = 1024; + std::size_t nullTerminatedCharPos = 0; + do { + auto errorCode = ::gethostname(hostname.data(), hostname.size() - 1U); + if (errorCode != 0) { +#ifdef CCT_MSVC + // In Windows, too small buffer returns an error WSAEFAULT + std::cout << "errorCode = " << errorCode << std::endl; + std::cerr << "WSAGetLastError() = " << WSAGetLastError() << std::endl; + if (WSAGetLastError() == 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..d3f30f28 --- /dev/null +++ b/src/tech/test/gethostname_test.cpp @@ -0,0 +1,13 @@ +#include "gethostname.hpp" + +#include + +namespace cct { +TEST(GetHostNameTest, Default) { + HostNameGetter hostNameGetter; + auto currentHostName = hostNameGetter.getHostName(); + + EXPECT_FALSE(currentHostName.empty()); + EXPECT_EQ(currentHostName.find('\0'), string::npos); +} +} // namespace cct \ No newline at end of file