diff options
-rw-r--r-- | .clang-tidy | 1 | ||||
-rw-r--r-- | src/mongo/bson/bson_validate_test.cpp | 18 | ||||
-rw-r--r-- | src/mongo/util/represent_as.h | 3 | ||||
-rw-r--r-- | src/mongo/util/represent_as_test.cpp | 9 |
4 files changed, 25 insertions, 6 deletions
diff --git a/.clang-tidy b/.clang-tidy index 5c5147b7563..5915b5812e6 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -35,6 +35,7 @@ Checks: '-*, performance-faster-string-find, performance-implicit-conversion-in-loop, performance-inefficient-algorithm, + bugprone-signed-char-misuse, bugprone-suspicious-string-compare, performance-for-range-copy, -bugprone-argument-comment, diff --git a/src/mongo/bson/bson_validate_test.cpp b/src/mongo/bson/bson_validate_test.cpp index d377d380009..5a2314b8882 100644 --- a/src/mongo/bson/bson_validate_test.cpp +++ b/src/mongo/bson/bson_validate_test.cpp @@ -580,11 +580,21 @@ TEST(BSONValidateFast, BoolValuesAreValidated) { ASSERT_OK(validateBSON(obj)); const BSONElement x = obj["x"]; // Legal, because we know that the BufBuilder gave - // us back some heap memory, which isn't oringinally const. + // us back some heap memory, which isn't originally const. auto writable = const_cast<char*>(x.value()); - for (int val = std::numeric_limits<char>::min(); - val != (int(std::numeric_limits<char>::max()) + 1); - ++val) { + // The next few lines may look non-optimal and strange, however the assignment of a + // signed char to int is not safe, generally speaking. Recall that the numeric_limits + // library returns the limit as the type requested and that the signedness of char is + // platform-dependent. We know in this case that all values of a byte need to be + // tested, so we can hardcode the limits ourselves to sidestep casting issues and + // warnings from clang-tidy. + // + // Loop iteration will be the following for platforms with: + // signed char: -128->127 + // unsigned char: 128->255, 0->127 + int signed_char_min = -128; + int signed_char_max = 127; + for (int val = signed_char_min; val != signed_char_max + 1; ++val) { *writable = static_cast<char>(val); if ((val == 0) || (val == 1)) { ASSERT_OK(validateBSON(obj)); diff --git a/src/mongo/util/represent_as.h b/src/mongo/util/represent_as.h index b491879f465..24468809ba8 100644 --- a/src/mongo/util/represent_as.h +++ b/src/mongo/util/represent_as.h @@ -208,6 +208,9 @@ boost::optional<Output> representAs(Input number) try { } else { return {}; } + } else if constexpr ((std::is_same_v<int, Output> && std::is_same_v<signed char, Input>) || + (std::is_same_v<int, Output> && std::is_same_v<char, Input>)) { + return static_cast<Output>(static_cast<unsigned char>(number)); } else { // If number is NaN and Output can also represent NaN, return NaN // Note: We need to specifically handle NaN here because of the way diff --git a/src/mongo/util/represent_as_test.cpp b/src/mongo/util/represent_as_test.cpp index d139dfa435b..bfd4a7bccc1 100644 --- a/src/mongo/util/represent_as_test.cpp +++ b/src/mongo/util/represent_as_test.cpp @@ -47,7 +47,9 @@ namespace { using namespace fmt::literals; // Char values -const signed char kCharMax = std::numeric_limits<signed char>::max(); +const char kCharMax = std::numeric_limits<char>::max(); +const signed char kSCharMax = std::numeric_limits<signed char>::max(); +const int kSCharMaxAsInt = kSCharMax; const int kCharMaxAsInt = kCharMax; // Unsigned char values @@ -430,9 +432,12 @@ TEST(RepresentAs, DoubleToDecimal128) { TEST(RepresentAs, PlatformDependent) { // signed char - ASSERT(*(representAs<int>(kCharMax)) == kCharMaxAsInt); + ASSERT(*(representAs<int>(kSCharMax)) == kSCharMaxAsInt); ASSERT(!(representAs<signed char>(kIntMax))); + // unspecified/bare char + ASSERT(*(representAs<int>(kCharMax)) == kCharMaxAsInt); + // unsigned char ASSERT(*(representAs<int>(kUCharMax)) == kUCharMaxAsInt); ASSERT(!(representAs<unsigned char>(kIntMin))); |