Skip to content

Commit

Permalink
refactor: ensure LDLogLevel is at least 32 bits wide (#415)
Browse files Browse the repository at this point in the history
C++ compilers (specifically clang) complain that
`static_cast<LDLogLevel>(some_value_outside_range_of_defined_enum_values)`
is UB. To fix this, add a value to the enum forcing the valid range to
be 32 bits.
  • Loading branch information
cwaldren-ld authored Jun 18, 2024
1 parent effc049 commit 811cece
Show file tree
Hide file tree
Showing 5 changed files with 16 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

#include <stdbool.h>
#include <stddef.h>
#include <stdint.h>

#ifdef __cplusplus
extern "C" { // only need to export C interface if
Expand All @@ -23,6 +24,8 @@ enum LDLogLevel {
LD_LOG_INFO = 1,
LD_LOG_WARN = 2,
LD_LOG_ERROR = 3,
LD_UNUSED_MAXVALUE = INT32_MAX /* Used to ensure the underlying type is
* at least 32 bits. */
};

/**
Expand All @@ -40,7 +43,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);
Expand Down
3 changes: 2 additions & 1 deletion libs/common/include/launchdarkly/logging/log_level.hpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#pragma once

#include <cstdint>
#include <ostream>

namespace launchdarkly {
Expand All @@ -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::int32_t {
kDebug = 0,
kInfo = 1,
kWarn = 2,
Expand Down
2 changes: 2 additions & 0 deletions libs/common/src/bindings/c/logging/log_level.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

#include <launchdarkly/logging/log_level.hpp>

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<launchdarkly::LogLevel>(level),
Expand Down
11 changes: 5 additions & 6 deletions libs/common/src/log_level.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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_;
}
}

Expand Down
5 changes: 2 additions & 3 deletions libs/common/tests/logging_c_binding_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<LDLogLevel>(4141), "unknown"));
ASSERT_STREQ("unknown", LDLogLevel_Name(LD_UNUSED_MAXVALUE, "unknown"));
}

TEST(LogLevelTests, LogLevelToEnum) {
constexpr auto unknown = static_cast<LDLogLevel>(4141);
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));
Expand Down

0 comments on commit 811cece

Please sign in to comment.