From eaf05340dc9e5367fdd43a9eea0c39a0aec44f16 Mon Sep 17 00:00:00 2001 From: "Allan L. Bazinet" Date: Tue, 5 Apr 2016 10:50:21 -0700 Subject: [PATCH 1/2] Support uint32 as flag type MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit From #99, “Given that there is an unsigned flag type for 64-bit integers, it is probably reasonable to request/expect the same for 32-bit integers.” --- src/gflags.cc | 41 +++++++++++++++++++++++++++++++----- src/gflags.h.in | 6 ++++++ src/gflags_declare.h.in | 3 +++ src/gflags_ns.h.in | 1 + test/gflags_unittest.cc | 46 ++++++++++++++++++++++++++++++----------- 5 files changed, 80 insertions(+), 17 deletions(-) diff --git a/src/gflags.cc b/src/gflags.cc index 85b1ef7..9881a80 100644 --- a/src/gflags.cc +++ b/src/gflags.cc @@ -208,11 +208,12 @@ class FlagValue { enum ValueType { FV_BOOL = 0, FV_INT32 = 1, - FV_INT64 = 2, - FV_UINT64 = 3, - FV_DOUBLE = 4, - FV_STRING = 5, - FV_MAX_INDEX = 5, + FV_UINT32 = 2, + FV_INT64 = 3, + FV_UINT64 = 4, + FV_DOUBLE = 5, + FV_STRING = 6, + FV_MAX_INDEX = 7, }; const char* TypeName() const; bool Equal(const FlagValue& x) const; @@ -260,6 +261,7 @@ FlagValue::~FlagValue() { switch (type_) { case FV_BOOL: delete reinterpret_cast(value_buffer_); break; case FV_INT32: delete reinterpret_cast(value_buffer_); break; + case FV_UINT32: delete reinterpret_cast(value_buffer_); break; case FV_INT64: delete reinterpret_cast(value_buffer_); break; case FV_UINT64: delete reinterpret_cast(value_buffer_); break; case FV_DOUBLE: delete reinterpret_cast(value_buffer_); break; @@ -308,6 +310,16 @@ bool FlagValue::ParseFrom(const char* value) { SET_VALUE_AS(int32, static_cast(r)); return true; } + case FV_UINT32: { + while (*value == ' ') value++; + if (*value == '-') return false; // negative number + const uint64 r = strtou64(value, &end, base); + if (errno || end != value + strlen(value)) return false; // bad parse + if (static_cast(r) != r) // worked, but number out of range + return false; + SET_VALUE_AS(uint32, static_cast(r)); + return true; + } case FV_INT64: { const int64 r = strto64(value, &end, base); if (errno || end != value + strlen(value)) return false; // bad parse @@ -343,6 +355,9 @@ string FlagValue::ToString() const { case FV_INT32: snprintf(intbuf, sizeof(intbuf), "%" PRId32, VALUE_AS(int32)); return intbuf; + case FV_UINT32: + snprintf(intbuf, sizeof(intbuf), "%" PRIu32, VALUE_AS(uint32)); + return intbuf; case FV_INT64: snprintf(intbuf, sizeof(intbuf), "%" PRId64, VALUE_AS(int64)); return intbuf; @@ -369,6 +384,9 @@ bool FlagValue::Validate(const char* flagname, case FV_INT32: return reinterpret_cast( validate_fn_proto)(flagname, VALUE_AS(int32)); + case FV_UINT32: + return reinterpret_cast( + validate_fn_proto)(flagname, VALUE_AS(uint32)); case FV_INT64: return reinterpret_cast( validate_fn_proto)(flagname, VALUE_AS(int64)); @@ -391,6 +409,7 @@ const char* FlagValue::TypeName() const { static const char types[] = "bool\0xx" "int32\0x" + "uin32\0x" "int64\0x" "uint64\0" "double\0" @@ -409,6 +428,7 @@ bool FlagValue::Equal(const FlagValue& x) const { switch (type_) { case FV_BOOL: return VALUE_AS(bool) == OTHER_VALUE_AS(x, bool); case FV_INT32: return VALUE_AS(int32) == OTHER_VALUE_AS(x, int32); + case FV_UINT32: return VALUE_AS(uint32) == OTHER_VALUE_AS(x, uint32); case FV_INT64: return VALUE_AS(int64) == OTHER_VALUE_AS(x, int64); case FV_UINT64: return VALUE_AS(uint64) == OTHER_VALUE_AS(x, uint64); case FV_DOUBLE: return VALUE_AS(double) == OTHER_VALUE_AS(x, double); @@ -422,6 +442,7 @@ FlagValue* FlagValue::New() const { switch (type_) { case FV_BOOL: return new FlagValue(new bool(false), type, true); case FV_INT32: return new FlagValue(new int32(0), type, true); + case FV_UINT32: return new FlagValue(new uint32(0), type, true); case FV_INT64: return new FlagValue(new int64(0), type, true); case FV_UINT64: return new FlagValue(new uint64(0), type, true); case FV_DOUBLE: return new FlagValue(new double(0.0), type, true); @@ -435,6 +456,7 @@ void FlagValue::CopyFrom(const FlagValue& x) { switch (type_) { case FV_BOOL: SET_VALUE_AS(bool, OTHER_VALUE_AS(x, bool)); break; case FV_INT32: SET_VALUE_AS(int32, OTHER_VALUE_AS(x, int32)); break; + case FV_UINT32: SET_VALUE_AS(uint32, OTHER_VALUE_AS(x, uint32)); break; case FV_INT64: SET_VALUE_AS(int64, OTHER_VALUE_AS(x, int64)); break; case FV_UINT64: SET_VALUE_AS(uint64, OTHER_VALUE_AS(x, uint64)); break; case FV_DOUBLE: SET_VALUE_AS(double, OTHER_VALUE_AS(x, double)); break; @@ -451,6 +473,7 @@ int FlagValue::ValueSize() const { static const uint8 valuesize[] = { sizeof(bool), sizeof(int32), + sizeof(uint32), sizeof(int64), sizeof(uint64), sizeof(double), @@ -1781,6 +1804,7 @@ bool ReadFromFlagsFile(const string& filename, const char* prog_name, // -------------------------------------------------------------------- // BoolFromEnv() // Int32FromEnv() +// Uint32FromEnv() // Int64FromEnv() // Uint64FromEnv() // DoubleFromEnv() @@ -1797,6 +1821,9 @@ bool BoolFromEnv(const char *v, bool dflt) { int32 Int32FromEnv(const char *v, int32 dflt) { return GetFromEnv(v, "int32", dflt); } +uint32 Uint32FromEnv(const char *v, uint32 dflt) { + return GetFromEnv(v, "uint32", dflt); +} int64 Int64FromEnv(const char *v, int64 dflt) { return GetFromEnv(v, "int64", dflt); } @@ -1840,6 +1867,10 @@ bool RegisterFlagValidator(const int32* flag, bool (*validate_fn)(const char*, int32)) { return AddFlagValidator(flag, reinterpret_cast(validate_fn)); } +bool RegisterFlagValidator(const uint32* flag, + bool (*validate_fn)(const char*, uint32)) { + return AddFlagValidator(flag, reinterpret_cast(validate_fn)); +} bool RegisterFlagValidator(const int64* flag, bool (*validate_fn)(const char*, int64)) { return AddFlagValidator(flag, reinterpret_cast(validate_fn)); diff --git a/src/gflags.h.in b/src/gflags.h.in index 88ab1aa..03b7776 100644 --- a/src/gflags.h.in +++ b/src/gflags.h.in @@ -128,6 +128,7 @@ namespace GFLAGS_NAMESPACE { // validator is already registered for this flag). extern GFLAGS_DLL_DECL bool RegisterFlagValidator(const bool* flag, bool (*validate_fn)(const char*, bool)); extern GFLAGS_DLL_DECL bool RegisterFlagValidator(const int32* flag, bool (*validate_fn)(const char*, int32)); +extern GFLAGS_DLL_DECL bool RegisterFlagValidator(const uint32* flag, bool (*validate_fn)(const char*, uint32)); extern GFLAGS_DLL_DECL bool RegisterFlagValidator(const int64* flag, bool (*validate_fn)(const char*, int64)); extern GFLAGS_DLL_DECL bool RegisterFlagValidator(const uint64* flag, bool (*validate_fn)(const char*, uint64)); extern GFLAGS_DLL_DECL bool RegisterFlagValidator(const double* flag, bool (*validate_fn)(const char*, double)); @@ -313,6 +314,7 @@ extern GFLAGS_DLL_DECL bool ReadFromFlagsFile(const std::string& filename, const extern GFLAGS_DLL_DECL bool BoolFromEnv(const char *varname, bool defval); extern GFLAGS_DLL_DECL int32 Int32FromEnv(const char *varname, int32 defval); +extern GFLAGS_DLL_DECL uint32 Uint32FromEnv(const char *varname, uint32 defval); extern GFLAGS_DLL_DECL int64 Int64FromEnv(const char *varname, int64 defval); extern GFLAGS_DLL_DECL uint64 Uint64FromEnv(const char *varname, uint64 defval); extern GFLAGS_DLL_DECL double DoubleFromEnv(const char *varname, double defval); @@ -508,6 +510,10 @@ GFLAGS_DLL_DECL bool IsBoolFlag(bool from); DEFINE_VARIABLE(GFLAGS_NAMESPACE::int32, I, \ name, val, txt) +#define DEFINE_uint32(name,val, txt) \ + DEFINE_VARIABLE(GFLAGS_NAMESPACE::uint32, U, \ + name, val, txt) + #define DEFINE_int64(name, val, txt) \ DEFINE_VARIABLE(GFLAGS_NAMESPACE::int64, I64, \ name, val, txt) diff --git a/src/gflags_declare.h.in b/src/gflags_declare.h.in index 279db24..a8b76f7 100644 --- a/src/gflags_declare.h.in +++ b/src/gflags_declare.h.in @@ -120,6 +120,9 @@ typedef std::string clstring; #define DECLARE_int32(name) \ DECLARE_VARIABLE(::GFLAGS_NAMESPACE::int32, I, name) +#define DECLARE_uint32(name) \ + DECLARE_VARIABLE(::GFLAGS_NAMESPACE::uint32, U, name) + #define DECLARE_int64(name) \ DECLARE_VARIABLE(::GFLAGS_NAMESPACE::int64, I64, name) diff --git a/src/gflags_ns.h.in b/src/gflags_ns.h.in index f692666..ef6ac29 100644 --- a/src/gflags_ns.h.in +++ b/src/gflags_ns.h.in @@ -77,6 +77,7 @@ using GFLAGS_NAMESPACE::AppendFlagsIntoFile; using GFLAGS_NAMESPACE::ReadFromFlagsFile; using GFLAGS_NAMESPACE::BoolFromEnv; using GFLAGS_NAMESPACE::Int32FromEnv; +using GFLAGS_NAMESPACE::Uint32FromEnv; using GFLAGS_NAMESPACE::Int64FromEnv; using GFLAGS_NAMESPACE::Uint64FromEnv; using GFLAGS_NAMESPACE::DoubleFromEnv; diff --git a/test/gflags_unittest.cc b/test/gflags_unittest.cc index 427af8c..045d012 100755 --- a/test/gflags_unittest.cc +++ b/test/gflags_unittest.cc @@ -75,6 +75,7 @@ DECLARE_string(tryfromenv); // in gflags.cc DEFINE_bool(test_bool, false, "tests bool-ness"); DEFINE_int32(test_int32, -1, ""); DEFINE_int64(test_int64, -2, ""); +DEFINE_uint32(test_uint32, 1, ""); DEFINE_uint64(test_uint64, 2, ""); DEFINE_double(test_double, -1.0, ""); DEFINE_string(test_string, "initial", ""); @@ -115,6 +116,7 @@ DEFINE_string(changeable_string_var, ChangeableString(), ""); DEFINE_bool(unused_bool, true, "unused bool-ness"); DEFINE_int32(unused_int32, -1001, ""); DEFINE_int64(unused_int64, -2001, ""); +DEFINE_uint32(unused_uint32, 1000, ""); DEFINE_uint64(unused_uint64, 2000, ""); DEFINE_double(unused_double, -1000.0, ""); DEFINE_string(unused_string, "unused", ""); @@ -277,6 +279,7 @@ TEST(FlagTypes, FlagTypes) { AssertIsType(FLAGS_test_bool); AssertIsType(FLAGS_test_int32); AssertIsType(FLAGS_test_int64); + AssertIsType(FLAGS_test_uint32); AssertIsType(FLAGS_test_uint64); AssertIsType(FLAGS_test_double); AssertIsType(FLAGS_test_string); @@ -591,11 +594,15 @@ TEST(SetFlagValueTest, IllegalValues) { FLAGS_test_bool = true; FLAGS_test_int32 = 119; FLAGS_test_int64 = 1191; - FLAGS_test_uint64 = 11911; + FLAGS_test_uint32 = 11911; + FLAGS_test_uint64 = 119111; EXPECT_EQ("", SetCommandLineOption("test_bool", "12")); + EXPECT_EQ("", + SetCommandLineOption("test_uint32", "1970")); + EXPECT_EQ("", SetCommandLineOption("test_int32", "7000000000000")); @@ -609,6 +616,7 @@ TEST(SetFlagValueTest, IllegalValues) { EXPECT_EQ("", SetCommandLineOption("test_bool", "")); EXPECT_EQ("", SetCommandLineOption("test_int32", "")); EXPECT_EQ("", SetCommandLineOption("test_int64", "")); + EXPECT_EQ("", SetCommandLineOption("test_uint32", "")); EXPECT_EQ("", SetCommandLineOption("test_uint64", "")); EXPECT_EQ("", SetCommandLineOption("test_double", "")); EXPECT_EQ("test_string set to \n", SetCommandLineOption("test_string", "")); @@ -616,7 +624,8 @@ TEST(SetFlagValueTest, IllegalValues) { EXPECT_TRUE(FLAGS_test_bool); EXPECT_EQ(119, FLAGS_test_int32); EXPECT_EQ(1191, FLAGS_test_int64); - EXPECT_EQ(11911, FLAGS_test_uint64); + EXPECT_EQ(11911, FLAGS_test_uint32); + EXPECT_EQ(119111, FLAGS_test_uint64); } @@ -669,14 +678,19 @@ TEST(FromEnvTest, LegalValues) { EXPECT_EQ(-1, Int32FromEnv("INT_VAL2", 10)); EXPECT_EQ(10, Int32FromEnv("INT_VAL_UNKNOWN", 10)); - setenv("INT_VAL3", "1099511627776", 1); + setenv("INT_VAL3", "4294967295", 1); + EXPECT_EQ(1, Uint32FromEnv("INT_VAL1", 10)); + EXPECT_EQ(4294967295L, Uint32FromEnv("INT_VAL3", 30)); + EXPECT_EQ(10, Uint32FromEnv("INT_VAL_UNKNOWN", 10)); + + setenv("INT_VAL4", "1099511627776", 1); EXPECT_EQ(1, Int64FromEnv("INT_VAL1", 20)); EXPECT_EQ(-1, Int64FromEnv("INT_VAL2", 20)); - EXPECT_EQ(1099511627776LL, Int64FromEnv("INT_VAL3", 20)); + EXPECT_EQ(1099511627776LL, Int64FromEnv("INT_VAL4", 20)); EXPECT_EQ(20, Int64FromEnv("INT_VAL_UNKNOWN", 20)); EXPECT_EQ(1, Uint64FromEnv("INT_VAL1", 30)); - EXPECT_EQ(1099511627776ULL, Uint64FromEnv("INT_VAL3", 30)); + EXPECT_EQ(1099511627776ULL, Uint64FromEnv("INT_VAL4", 30)); EXPECT_EQ(30, Uint64FromEnv("INT_VAL_UNKNOWN", 30)); // I pick values here that can be easily represented exactly in floating-point @@ -712,6 +726,11 @@ TEST(FromEnvDeathTest, IllegalValues) { EXPECT_DEATH(Int32FromEnv("INT_BAD3", 10), "error parsing env variable"); EXPECT_DEATH(Int32FromEnv("INT_BAD4", 10), "error parsing env variable"); + EXPECT_DEATH(Uint32FromEnv("INT_BAD1", 10), "error parsing env variable"); + EXPECT_DEATH(Uint32FromEnv("INT_BAD2", 10), "error parsing env variable"); + EXPECT_DEATH(Uint32FromEnv("INT_BAD3", 10), "error parsing env variable"); + EXPECT_DEATH(Uint32FromEnv("INT_BAD4", 10), "error parsing env variable"); + setenv("BIGINT_BAD1", "18446744073709551616000", 1); EXPECT_DEATH(Int64FromEnv("INT_BAD1", 20), "error parsing env variable"); EXPECT_DEATH(Int64FromEnv("INT_BAD3", 20), "error parsing env variable"); @@ -815,9 +834,10 @@ TEST(FlagSaverTest, CanSaveVariousTypedFlagValues) { // Initializes the flags. FLAGS_test_bool = false; FLAGS_test_int32 = -1; - FLAGS_test_int64 = -2; - FLAGS_test_uint64 = 3; - FLAGS_test_double = 4.0; + FLAGS_test_uint32 = 2; + FLAGS_test_int64 = -3; + FLAGS_test_uint64 = 4; + FLAGS_test_double = 5.0; FLAGS_test_string = "good"; // Saves the flag states. @@ -827,8 +847,9 @@ TEST(FlagSaverTest, CanSaveVariousTypedFlagValues) { // Modifies the flags. FLAGS_test_bool = true; FLAGS_test_int32 = -5; - FLAGS_test_int64 = -6; - FLAGS_test_uint64 = 7; + FLAGS_test_uint32 = 6; + FLAGS_test_int64 = -7; + FLAGS_test_uint64 = 8; FLAGS_test_double = 8.0; FLAGS_test_string = "bad"; @@ -838,8 +859,9 @@ TEST(FlagSaverTest, CanSaveVariousTypedFlagValues) { // Verifies the flag values were restored. EXPECT_FALSE(FLAGS_test_bool); EXPECT_EQ(-1, FLAGS_test_int32); - EXPECT_EQ(-2, FLAGS_test_int64); - EXPECT_EQ(3, FLAGS_test_uint64); + EXPECT_EQ(2, FLAGS_test_uint32); + EXPECT_EQ(-3, FLAGS_test_int64); + EXPECT_EQ(4, FLAGS_test_uint64); EXPECT_DOUBLE_EQ(4.0, FLAGS_test_double); EXPECT_EQ("good", FLAGS_test_string); } From 81d8a9234b285016e46bad8ca328bd29d11fd29b Mon Sep 17 00:00:00 2001 From: "Allan L. Bazinet" Date: Tue, 5 Apr 2016 10:56:57 -0700 Subject: [PATCH 2/2] Correct FV_MAX_INDEX --- src/gflags.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/gflags.cc b/src/gflags.cc index 9881a80..c4a689d 100644 --- a/src/gflags.cc +++ b/src/gflags.cc @@ -213,7 +213,7 @@ class FlagValue { FV_UINT64 = 4, FV_DOUBLE = 5, FV_STRING = 6, - FV_MAX_INDEX = 7, + FV_MAX_INDEX = 6, }; const char* TypeName() const; bool Equal(const FlagValue& x) const;