diff --git a/base/option.h b/base/option.h
index 02c473de..0c49626d 100644
--- a/base/option.h
+++ b/base/option.h
@@ -15,13 +15,15 @@
#include
#include
#include
+#include
namespace base {
namespace detail {
// Parse `text` into `out`. Returns false (leaving `out` untouched) when the
-// text is not a valid value for T. Covers bool, the integral and floating
-// types, and const char* (which is bound to the source string, not copied).
+// text is not a valid value for T. Covers bool and the integral and floating
+// types; a const char* option does not come through here, because it has to
+// own what it is given (Option::ParseFromString).
template
inline bool ParseOption(const char* text, T& out) {
if constexpr (base::is_same_v) {
@@ -44,9 +46,6 @@ inline bool ParseOption(const char* text, T& out) {
return true;
}
return false;
- } else if constexpr (base::is_same_v) {
- out = text;
- return true;
} else if constexpr (base::is_integral_v) {
i64 v = 0;
if (!base::ParseInteger(text, v, /*base_radix=*/0)) return false;
@@ -124,6 +123,18 @@ class BASE_EXPORT OptionBase : public InitChain {
// programmatic default. Declare options at namespace scope, not as
// function-local statics: InitOptionsFromEnv() can only populate options that
// were already constructed (and thus registered) by the time it runs.
+namespace detail {
+// The buffer a const char* option keeps its value in. Empty for every other T,
+// and empty bases/members cost nothing here, so an Option is the same
+// size it always was.
+template
+struct OwnedText {};
+template <>
+struct OwnedText {
+ StringU8 text;
+};
+} // namespace detail
+
template
class Option : public OptionBase {
public:
@@ -149,14 +160,32 @@ class Option : public OptionBase {
protected:
bool ParseFromString(const char* text) override {
- return detail::ParseOption(text, value_);
+ if constexpr (base::is_same_v) {
+ // A string option owns its value. What SetFromString is handed is
+ // whatever its caller had at the time -- an environment variable read
+ // into a local, one line of a config file, a name off a command line --
+ // and none of those are still there when the option is next read, so
+ // pointing at the caller's buffer is a use-after-free waiting for the
+ // first reader.
+ if (!text) return false;
+ owned_.text = reinterpret_cast(text);
+ value_ = reinterpret_cast(owned_.text.c_str());
+ return true;
+ } else {
+ return detail::ParseOption(text, value_);
+ }
}
- void RestoreDefault() override { value_ = default_; }
+ void RestoreDefault() override {
+ value_ = default_;
+ // The default is a literal the option does not own; drop what it did.
+ if constexpr (base::is_same_v) owned_.text.clear();
+ }
private:
T value_;
const T default_;
+ [[no_unique_address]] detail::OwnedText owned_;
};
// Populate every registered option that names an environment variable from the
diff --git a/base/option_test.cc b/base/option_test.cc
new file mode 100644
index 00000000..496733a6
--- /dev/null
+++ b/base/option_test.cc
@@ -0,0 +1,86 @@
+// Copyright (C) 2026 Vincent Hengel.
+// For licensing information see LICENSE at the root of this distribution.
+
+#include
+#include
+#include
+
+#include
+
+namespace {
+
+base::Option kFlag{"opt.flag", false, "BASE_OPT_FLAG", "a boolean"};
+base::Option kCount{"opt.count", 7, "BASE_OPT_COUNT", "a number"};
+base::Option kScale{"opt.scale", 1.0f, "BASE_OPT_SCALE"};
+base::Option kName{"opt.name", "default", "BASE_OPT_NAME"};
+
+class OptionTest : public ::testing::Test {
+ protected:
+ void SetUp() override { TearDown(); }
+ void TearDown() override {
+ kFlag.Reset();
+ kCount.Reset();
+ kScale.Reset();
+ kName.Reset();
+ base::DeleteEnvironmentVariable(u8"BASE_OPT_FLAG");
+ base::DeleteEnvironmentVariable(u8"BASE_OPT_COUNT");
+ base::DeleteEnvironmentVariable(u8"BASE_OPT_SCALE");
+ base::DeleteEnvironmentVariable(u8"BASE_OPT_NAME");
+ }
+};
+
+TEST_F(OptionTest, ParsesEachType) {
+ EXPECT_TRUE(kFlag.SetFromString("on"));
+ EXPECT_TRUE(kCount.SetFromString("128"));
+ EXPECT_TRUE(kScale.SetFromString("0.25"));
+ EXPECT_TRUE(kFlag.get());
+ EXPECT_EQ(kCount.get(), 128);
+ EXPECT_FLOAT_EQ(kScale.get(), 0.25f);
+}
+
+// The whole reason a string option copies: SetFromString is handed whatever
+// its caller had at the time, and the option is read long after that is gone.
+TEST_F(OptionTest, StringValueOutlivesTheCallersBuffer) {
+ {
+ base::String text("from-a-temporary");
+ ASSERT_TRUE(kName.SetFromString(text.c_str()));
+ }
+ EXPECT_STREQ(kName.get(), "from-a-temporary");
+}
+
+// InitOptionsFromEnv reads each variable into a local, which is the same trap
+// one call further out.
+TEST_F(OptionTest, EnvironmentOverrideOutlivesTheRead) {
+ ASSERT_TRUE(base::SetEnvironmentVariable(u8"BASE_OPT_NAME", u8"from-the-env"));
+ ASSERT_TRUE(base::SetEnvironmentVariable(u8"BASE_OPT_COUNT", u8"42"));
+
+ EXPECT_GE(base::InitOptionsFromEnv(), 2u);
+ EXPECT_STREQ(kName.get(), "from-the-env");
+ EXPECT_EQ(kCount.get(), 42);
+ EXPECT_TRUE(kName.overridden());
+}
+
+TEST_F(OptionTest, ResetDropsTheOwnedString) {
+ {
+ base::String text("temporary");
+ ASSERT_TRUE(kName.SetFromString(text.c_str()));
+ }
+ kName.Reset();
+ EXPECT_STREQ(kName.get(), "default");
+ EXPECT_FALSE(kName.overridden());
+}
+
+TEST_F(OptionTest, RejectsWhatDoesNotParse) {
+ ASSERT_TRUE(kCount.SetFromString("9"));
+ EXPECT_FALSE(kCount.SetFromString("not-a-number"));
+ EXPECT_EQ(kCount.get(), 9); // a bad value leaves the old one alone
+ EXPECT_FALSE(kName.SetFromString(nullptr));
+}
+
+// A string option pays for its buffer; nothing else should.
+TEST_F(OptionTest, NonStringOptionsCarryNoStorage) {
+ EXPECT_EQ(sizeof(base::Option), sizeof(base::Option));
+ EXPECT_LT(sizeof(base::Option), sizeof(base::Option));
+}
+
+} // namespace