From a7c325364e77e1ce65045d6a410bc5c2d3834ccc Mon Sep 17 00:00:00 2001 From: Casey Waldren Date: Wed, 12 Jun 2024 19:43:46 -0700 Subject: [PATCH 1/2] fix: ensure LDLogLevel is at least 32 bits wide --- .../launchdarkly/bindings/c/logging/log_level.h | 5 ++++- .../common/include/launchdarkly/logging/log_level.hpp | 3 ++- libs/common/src/bindings/c/logging/log_level.cpp | 2 ++ libs/common/src/log_level.cpp | 11 +++++------ libs/common/tests/logging_c_binding_test.cpp | 5 ++--- 5 files changed, 15 insertions(+), 11 deletions(-) diff --git a/libs/common/include/launchdarkly/bindings/c/logging/log_level.h b/libs/common/include/launchdarkly/bindings/c/logging/log_level.h index 41ba38a6a..7d4698acc 100644 --- a/libs/common/include/launchdarkly/bindings/c/logging/log_level.h +++ b/libs/common/include/launchdarkly/bindings/c/logging/log_level.h @@ -23,6 +23,8 @@ enum LDLogLevel { LD_LOG_INFO = 1, LD_LOG_WARN = 2, LD_LOG_ERROR = 3, + LD_LOG_MAXVAL = 0x7FFFFFFF /* This is not a valid log level, it is used to + * ensure the enum is at least 32 bits. */ }; /** @@ -40,7 +42,8 @@ LDLogLevel_Name(enum LDLogLevel level, char const* level_if_unknown); * @param level Name of level. * @param level_if_unknown Default level to return if the level wasn't * recognized. - * @return LDLogLevel matching the name, or level_if_unknown if not recognized. + * @return LDLogLevel matching the name, or level_if_unknown if not + * recognized. */ LD_EXPORT(enum LDLogLevel) LDLogLevel_Enum(char const* level, enum LDLogLevel level_if_unknown); diff --git a/libs/common/include/launchdarkly/logging/log_level.hpp b/libs/common/include/launchdarkly/logging/log_level.hpp index 5338b2908..b39d0e71a 100644 --- a/libs/common/include/launchdarkly/logging/log_level.hpp +++ b/libs/common/include/launchdarkly/logging/log_level.hpp @@ -1,5 +1,6 @@ #pragma once +#include #include namespace launchdarkly { @@ -8,7 +9,7 @@ namespace launchdarkly { * severity. The values must not be changed to ensure backwards compatibility * with the C API. */ -enum class LogLevel { +enum class LogLevel : std::uint32_t { kDebug = 0, kInfo = 1, kWarn = 2, diff --git a/libs/common/src/bindings/c/logging/log_level.cpp b/libs/common/src/bindings/c/logging/log_level.cpp index e73e9c267..16f3deb9b 100644 --- a/libs/common/src/bindings/c/logging/log_level.cpp +++ b/libs/common/src/bindings/c/logging/log_level.cpp @@ -2,6 +2,8 @@ #include +static_assert(sizeof(LDLogLevel) >= 4, "LDLogLevel must be at least 32 bits"); + LD_EXPORT(char const*) LDLogLevel_Name(LDLogLevel level, char const* name_if_unknown) { return GetLogLevelName(static_cast(level), diff --git a/libs/common/src/log_level.cpp b/libs/common/src/log_level.cpp index d708d056f..d80713ae7 100644 --- a/libs/common/src/log_level.cpp +++ b/libs/common/src/log_level.cpp @@ -11,13 +11,12 @@ char const* GetLogLevelName(LogLevel level, char const* default_) { return "debug"; case LogLevel::kInfo: return "info"; - case LogLevel::kWarn: { + case LogLevel::kWarn: return "warn"; - case LogLevel::kError: - return "error"; - default: - return default_; - } + case LogLevel::kError: + return "error"; + default: + return default_; } } diff --git a/libs/common/tests/logging_c_binding_test.cpp b/libs/common/tests/logging_c_binding_test.cpp index 429b6d012..d3fc2ff42 100644 --- a/libs/common/tests/logging_c_binding_test.cpp +++ b/libs/common/tests/logging_c_binding_test.cpp @@ -7,12 +7,11 @@ TEST(LogLevelTests, LogLevelToString) { ASSERT_STREQ("info", LDLogLevel_Name(LD_LOG_INFO, "unknown")); ASSERT_STREQ("warn", LDLogLevel_Name(LD_LOG_WARN, "unknown")); ASSERT_STREQ("error", LDLogLevel_Name(LD_LOG_ERROR, "unknown")); - ASSERT_STREQ("unknown", - LDLogLevel_Name(static_cast(4141), "unknown")); + ASSERT_STREQ("unknown", LDLogLevel_Name(LD_LOG_MAXVAL, "unknown")); } TEST(LogLevelTests, LogLevelToEnum) { - constexpr auto unknown = static_cast(4141); + constexpr auto unknown = LD_LOG_MAXVAL; ASSERT_EQ(LD_LOG_DEBUG, LDLogLevel_Enum("debug", unknown)); ASSERT_EQ(LD_LOG_INFO, LDLogLevel_Enum("info", unknown)); ASSERT_EQ(LD_LOG_WARN, LDLogLevel_Enum("warn", unknown)); From e95c4f65fa92a873a93a88a7cb58ea4b372c0122 Mon Sep 17 00:00:00 2001 From: Casey Waldren Date: Tue, 18 Jun 2024 09:37:24 -0700 Subject: [PATCH 2/2] use int32max --- .../include/launchdarkly/bindings/c/logging/log_level.h | 5 +++-- libs/common/include/launchdarkly/logging/log_level.hpp | 2 +- libs/common/tests/logging_c_binding_test.cpp | 4 ++-- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/libs/common/include/launchdarkly/bindings/c/logging/log_level.h b/libs/common/include/launchdarkly/bindings/c/logging/log_level.h index 7d4698acc..7c26079d5 100644 --- a/libs/common/include/launchdarkly/bindings/c/logging/log_level.h +++ b/libs/common/include/launchdarkly/bindings/c/logging/log_level.h @@ -8,6 +8,7 @@ #include #include +#include #ifdef __cplusplus extern "C" { // only need to export C interface if @@ -23,8 +24,8 @@ enum LDLogLevel { LD_LOG_INFO = 1, LD_LOG_WARN = 2, LD_LOG_ERROR = 3, - LD_LOG_MAXVAL = 0x7FFFFFFF /* This is not a valid log level, it is used to - * ensure the enum is at least 32 bits. */ + LD_UNUSED_MAXVALUE = INT32_MAX /* Used to ensure the underlying type is + * at least 32 bits. */ }; /** diff --git a/libs/common/include/launchdarkly/logging/log_level.hpp b/libs/common/include/launchdarkly/logging/log_level.hpp index b39d0e71a..9aa79f583 100644 --- a/libs/common/include/launchdarkly/logging/log_level.hpp +++ b/libs/common/include/launchdarkly/logging/log_level.hpp @@ -9,7 +9,7 @@ namespace launchdarkly { * severity. The values must not be changed to ensure backwards compatibility * with the C API. */ -enum class LogLevel : std::uint32_t { +enum class LogLevel : std::int32_t { kDebug = 0, kInfo = 1, kWarn = 2, diff --git a/libs/common/tests/logging_c_binding_test.cpp b/libs/common/tests/logging_c_binding_test.cpp index d3fc2ff42..bf01bb359 100644 --- a/libs/common/tests/logging_c_binding_test.cpp +++ b/libs/common/tests/logging_c_binding_test.cpp @@ -7,11 +7,11 @@ TEST(LogLevelTests, LogLevelToString) { ASSERT_STREQ("info", LDLogLevel_Name(LD_LOG_INFO, "unknown")); ASSERT_STREQ("warn", LDLogLevel_Name(LD_LOG_WARN, "unknown")); ASSERT_STREQ("error", LDLogLevel_Name(LD_LOG_ERROR, "unknown")); - ASSERT_STREQ("unknown", LDLogLevel_Name(LD_LOG_MAXVAL, "unknown")); + ASSERT_STREQ("unknown", LDLogLevel_Name(LD_UNUSED_MAXVALUE, "unknown")); } TEST(LogLevelTests, LogLevelToEnum) { - constexpr auto unknown = LD_LOG_MAXVAL; + LDLogLevel unknown = LD_UNUSED_MAXVALUE; ASSERT_EQ(LD_LOG_DEBUG, LDLogLevel_Enum("debug", unknown)); ASSERT_EQ(LD_LOG_INFO, LDLogLevel_Enum("info", unknown)); ASSERT_EQ(LD_LOG_WARN, LDLogLevel_Enum("warn", unknown));