From 51ee34754046069703a75e9455bec8d5d9152da4 Mon Sep 17 00:00:00 2001 From: Sage Weil Date: Wed, 19 Dec 2018 13:24:06 -0600 Subject: [PATCH] common/options: use new parse_timespan It's more complete and robust, and lives in ceph_time.h where it probably belongs. Signed-off-by: Sage Weil --- src/common/options.cc | 71 ++-------------------------------- src/test/common/test_config.cc | 68 ++++++++++++++++++++++++++++++-- 2 files changed, 68 insertions(+), 71 deletions(-) diff --git a/src/common/options.cc b/src/common/options.cc index 681c8a6153673..d1b7b88b957f9 100644 --- a/src/common/options.cc +++ b/src/common/options.cc @@ -124,72 +124,6 @@ int Option::validate(const Option::value_t &new_value, std::string *err) const return 0; } -namespace { -template -std::chrono::seconds -do_parse_duration(const char* unit, string val, - size_t start, size_t* new_start) -{ - auto found = val.find(unit, start); - if (found == val.npos) { - *new_start = start; - return Duration{0}; - } - val[found] = '\0'; - string err; - char* s = &val[start]; - auto intervals = strict_strtoll(s, 10, &err); - if (!err.empty()) { - throw invalid_argument(s); - } - auto secs = chrono::duration_cast(Duration{intervals}); - *new_start = found + strlen(unit); - return secs; -} - -std::chrono::seconds parse_duration(const std::string& s) -{ - using namespace std::chrono; - auto secs = 0s; - size_t start = 0; - size_t new_start = 0; - using days_t = duration>; - auto v = s; - v.erase(std::remove_if(begin(v), end(v), - [](char c){ return std::isspace(c);}), end(v)); - if (auto delta = do_parse_duration("days", v, start, &new_start); - delta.count()) { - start = new_start; - secs += delta; - } - if (auto delta = do_parse_duration("hours", v, start, &new_start); - delta.count()) { - start = new_start; - secs += delta; - } - if (auto delta = do_parse_duration("minutes", v, start, &new_start); - delta.count()) { - start = new_start; - secs += delta; - } - if (auto delta = do_parse_duration("seconds", v, start, &new_start); - delta.count()) { - start = new_start; - secs += delta; - } - if (new_start == 0) { - string err; - if (auto delta = std::chrono::seconds{strict_strtoll(s.c_str(), 10, &err)}; - err.empty()) { - secs += delta; - } else { - throw invalid_argument(err); - } - } - return secs; -} -} // anonymous namespace - int Option::parse_value( const std::string& raw_val, value_t *out, @@ -262,8 +196,9 @@ int Option::parse_value( *out = sz; } else if (type == Option::TYPE_SECS) { try { - *out = parse_duration(val); - } catch (const invalid_argument&) { + *out = parse_timespan(val); + } catch (const invalid_argument& e) { + *error_message = e.what(); return -EINVAL; } } else { diff --git a/src/test/common/test_config.cc b/src/test/common/test_config.cc index 116ddbb3880f0..d72552b219924 100644 --- a/src/test/common/test_config.cc +++ b/src/test/common/test_config.cc @@ -159,15 +159,77 @@ TEST(md_config_t, set_val) const string s{"1 days 2 hours 4 minutes"}; using days_t = duration>; auto expected = (duration_cast(days_t{1}) + - duration_cast(hours{2}) + - duration_cast(minutes{4})); + duration_cast(hours{2}) + + duration_cast(minutes{4})); EXPECT_EQ(0, conf.set_val("mgr_tick_period", - "1 days 2 hours 4 minutes", nullptr)); + "1 days 2 hours 4 minutes", nullptr)); EXPECT_EQ(expected.count(), conf.get_val("mgr_tick_period").count()); EXPECT_EQ(-EINVAL, conf.set_val("mgr_tick_period", "21 centuries", nullptr)); EXPECT_EQ(expected.count(), conf.get_val("mgr_tick_period").count()); } + using namespace std::chrono; + + using days_t = duration>; + + struct testcase { + std::string s; + std::chrono::seconds r; + }; + std::vector good = { + { "23"s, duration_cast(seconds{23}) }, + { " 23 "s, duration_cast(seconds{23}) }, + { " 23s "s, duration_cast(seconds{23}) }, + { " 23 s "s, duration_cast(seconds{23}) }, + { " 23 sec "s, duration_cast(seconds{23}) }, + { "23 second "s, duration_cast(seconds{23}) }, + { "23 seconds"s, duration_cast(seconds{23}) }, + { "2m5s"s, duration_cast(seconds{2*60+5}) }, + { "2 m 5 s "s, duration_cast(seconds{2*60+5}) }, + { "2 m5"s, duration_cast(seconds{2*60+5}) }, + { "2 min5"s, duration_cast(seconds{2*60+5}) }, + { "2 minutes 5"s, duration_cast(seconds{2*60+5}) }, + { "1w"s, duration_cast(seconds{3600*24*7}) }, + { "1wk"s, duration_cast(seconds{3600*24*7}) }, + { "1week"s, duration_cast(seconds{3600*24*7}) }, + { "1weeks"s, duration_cast(seconds{3600*24*7}) }, + { "1month"s, duration_cast(seconds{3600*24*30}) }, + { "1months"s, duration_cast(seconds{3600*24*30}) }, + { "1M"s, duration_cast(seconds{3600*24*30}) }, + { "1y"s, duration_cast(seconds{3600*24*365}) }, + { "1yr"s, duration_cast(seconds{3600*24*365}) }, + { "1year"s, duration_cast(seconds{3600*24*365}) }, + { "1years"s, duration_cast(seconds{3600*24*365}) }, + { "1d2h3m4s"s, + duration_cast(days_t{1}) + + duration_cast(hours{2}) + + duration_cast(minutes{3}) + + duration_cast(seconds{4}) }, + { "1 days 2 hours 4 minutes"s, + duration_cast(days_t{1}) + + duration_cast(hours{2}) + + duration_cast(minutes{4}) }, + }; + + for (auto& i : good) { + cout << "good: " << i.s << " -> " << i.r.count() << std::endl; + EXPECT_EQ(0, conf.set_val("mgr_tick_period", i.s, nullptr)); + EXPECT_EQ(i.r.count(), conf.get_val("mgr_tick_period").count()); + } + + std::vector bad = { + "12x", + "_ 12", + "1 2", + "21 centuries", + "1 y m", + }; + for (auto& i : bad) { + std::stringstream err; + EXPECT_EQ(-EINVAL, conf.set_val("mgr_tick_period", i, &err)); + cout << "bad: " << i << " -> " << err.str() << std::endl; + } + for (int i = 0; i < 100; ++i) { std::chrono::seconds j = std::chrono::seconds(rand()); string s = exact_timespan_str(j); -- 2.39.5