From f753b74f3760abcc7b8b74173f75cc5ae664488e Mon Sep 17 00:00:00 2001 From: Jan Fajerski Date: Tue, 20 Feb 2018 17:38:05 +0100 Subject: [PATCH] common/strtol: add strict_iecstrtoll; strict_sistrtoll uses base 10 This adds parsing of iec units (IEC units Ki, Mi, ...) besides the parsing of the usual unit multipliers (SI units K, M, ...). Both unit prefixes are parsed with base 2. While SI units should be parsed with base 10, this would likely confuse users. Signed-off-by: Jan Fajerski --- src/common/strtol.cc | 80 ++++++++++++++-- src/common/strtol.h | 5 + src/kv/RocksDBStore.cc | 4 +- src/mon/OSDMonitor.cc | 21 +++-- src/rgw/rgw_admin.cc | 2 +- src/test/objectstore_bench.cc | 2 +- src/test/strtol.cc | 168 ++++++++++++++++++++++++++++++--- src/tools/rados/rados.cc | 2 +- src/tools/rbd/ArgumentTypes.cc | 8 +- src/tools/rbd/action/Bench.cc | 2 +- 10 files changed, 257 insertions(+), 37 deletions(-) diff --git a/src/common/strtol.cc b/src/common/strtol.cc index 5721b63fb41d7..43a7de011faa5 100644 --- a/src/common/strtol.cc +++ b/src/common/strtol.cc @@ -16,6 +16,7 @@ #include #include +#include #include using std::ostringstream; @@ -129,14 +130,23 @@ float strict_strtof(const char *str, std::string *err) } template -T strict_si_cast(const char *str, std::string *err) +T strict_iec_cast(const char *str, std::string *err) { std::string s(str); if (s.empty()) { - *err = "strict_sistrtoll: value not specified"; + *err = "strict_iecstrtoll: value not specified"; return 0; } - const char &u = s.back(); + char &u = s.back(); + // consume 'i' if present, fail if "Bi" is passed + if ( u == 'i') { + s.pop_back(); + u = s.back(); + if (u == 'B') { + *err = "strict_iecstrtoll: illegal prefix \"Bi\""; + return 0; + } + } int m = 0; if (u == 'B') m = 0; @@ -162,28 +172,84 @@ T strict_si_cast(const char *str, std::string *err) long long ll = strict_strtoll(s.c_str(), 10, err); if (ll < 0 && !std::numeric_limits::is_signed) { - *err = "strict_sistrtoll: value should not be negative"; + *err = "strict_iecstrtoll: value should not be negative"; return 0; } if (static_cast(m) >= sizeof(T) * CHAR_BIT) { - *err = ("strict_sistrtoll: the SI prefix is too large for the designated " + *err = ("strict_iecstrtoll: the IEC prefix is too large for the designated " "type"); return 0; } using promoted_t = typename std::common_type::type; if (static_cast(ll) < static_cast(std::numeric_limits::min()) >> m) { - *err = "strict_sistrtoll: value seems to be too small"; + *err = "strict_iecstrtoll: value seems to be too small"; return 0; } if (static_cast(ll) > static_cast(std::numeric_limits::max()) >> m) { - *err = "strict_sistrtoll: value seems to be too large"; + *err = "strict_iecstrtoll: value seems to be too large"; return 0; } return (ll << m); } +template int strict_iec_cast(const char *str, std::string *err); +template long strict_iec_cast(const char *str, std::string *err); +template long long strict_iec_cast(const char *str, std::string *err); +template uint64_t strict_iec_cast(const char *str, std::string *err); +template uint32_t strict_iec_cast(const char *str, std::string *err); + +uint64_t strict_iecstrtoll(const char *str, std::string *err) +{ + return strict_iec_cast(str, err); +} + +template +T strict_si_cast(const char *str, std::string *err) +{ + std::string s(str); + if (s.empty()) { + *err = "strict_sistrtoll: value not specified"; + return 0; + } + char &u = s.back(); + int m = 0; + if (u == 'K') + m = 3; + else if (u == 'M') + m = 6; + else if (u == 'G') + m = 9; + else if (u == 'T') + m = 12; + else if (u == 'P') + m = 15; + else if (u == 'E') + m = 18; + + if (m >= 3) + s.pop_back(); + + long long ll = strict_strtoll(s.c_str(), 10, err); + if (ll < 0 && !std::numeric_limits::is_signed) { + *err = "strict_sistrtoll: value should not be negative"; + return 0; + } + using promoted_t = typename std::common_type::type; + if (static_cast(ll) < + static_cast(std::numeric_limits::min()) / pow (10, m)) { + *err = "strict_sistrtoll: value seems to be too small"; + return 0; + } + if (static_cast(ll) > + static_cast(std::numeric_limits::max()) / pow (10, m)) { + *err = "strict_sistrtoll: value seems to be too large"; + return 0; + } + return (ll * pow (10, m)); +} + template int strict_si_cast(const char *str, std::string *err); template long strict_si_cast(const char *str, std::string *err); template long long strict_si_cast(const char *str, std::string *err); diff --git a/src/common/strtol.h b/src/common/strtol.h index 810273ebd2325..a7c0cc220b6f9 100644 --- a/src/common/strtol.h +++ b/src/common/strtol.h @@ -28,6 +28,11 @@ double strict_strtod(const char *str, std::string *err); float strict_strtof(const char *str, std::string *err); +uint64_t strict_iecstrtoll(const char *str, std::string *err); + +template +T strict_iec_cast(const char *str, std::string *err); + uint64_t strict_sistrtoll(const char *str, std::string *err); template diff --git a/src/kv/RocksDBStore.cc b/src/kv/RocksDBStore.cc index 26ee903ef997b..779441ce97533 100644 --- a/src/kv/RocksDBStore.cc +++ b/src/kv/RocksDBStore.cc @@ -212,14 +212,14 @@ int RocksDBStore::tryInterpret(const string &key, const string &val, rocksdb::Op { if (key == "compaction_threads") { std::string err; - int f = strict_sistrtoll(val.c_str(), &err); + int f = strict_iecstrtoll(val.c_str(), &err); if (!err.empty()) return -EINVAL; //Low priority threadpool is used for compaction opt.env->SetBackgroundThreads(f, rocksdb::Env::Priority::LOW); } else if (key == "flusher_threads") { std::string err; - int f = strict_sistrtoll(val.c_str(), &err); + int f = strict_iecstrtoll(val.c_str(), &err); if (!err.empty()) return -EINVAL; //High priority threadpool is used for flusher diff --git a/src/mon/OSDMonitor.cc b/src/mon/OSDMonitor.cc index 0e14ad6443b74..924db810cb5c0 100644 --- a/src/mon/OSDMonitor.cc +++ b/src/mon/OSDMonitor.cc @@ -5307,7 +5307,7 @@ int OSDMonitor::normalize_profile(const string& profilename, auto it = profile.find("stripe_unit"); if (it != profile.end()) { string err_str; - uint32_t stripe_unit = strict_si_cast(it->second.c_str(), &err_str); + uint32_t stripe_unit = strict_iecstrtoll(it->second.c_str(), &err_str); if (!err_str.empty()) { *ss << "could not parse stripe_unit '" << it->second << "': " << err_str << std::endl; @@ -5581,7 +5581,7 @@ int OSDMonitor::prepare_pool_stripe_width(const unsigned pool_type, auto it = profile.find("stripe_unit"); if (it != profile.end()) { string err_str; - stripe_unit = strict_si_cast(it->second.c_str(), &err_str); + stripe_unit = strict_iecstrtoll(it->second.c_str(), &err_str); assert(err_str.empty()); } *stripe_width = data_chunks * @@ -10950,11 +10950,18 @@ bool OSDMonitor::prepare_command_impl(MonOpRequestRef op, // val could contain unit designations, so we treat as a string string val; cmd_getval(cct, cmdmap, "val", val); - stringstream tss; - int64_t value = unit_to_bytesize(val, &tss); - if (value < 0) { - ss << "error parsing value '" << value << "': " << tss.str(); - err = value; + string tss; + int64_t value; + if (field == "max_objects") { + value = strict_sistrtoll(val.c_str(), &tss); + } else if (field == "max_bytes") { + value = strict_iecstrtoll(val.c_str(), &tss); + } else { + assert(0 == "unrecognized option"); + } + if (!tss.empty()) { + ss << "error parsing value '" << val << "': " << tss; + err = -EINVAL; goto reply; } diff --git a/src/rgw/rgw_admin.cc b/src/rgw/rgw_admin.cc index 949fc376f17a4..10a619d998dc0 100644 --- a/src/rgw/rgw_admin.cc +++ b/src/rgw/rgw_admin.cc @@ -2635,7 +2635,7 @@ int main(int argc, const char **argv) return EINVAL; } } else if (ceph_argparse_witharg(args, i, &val, "--max-size", (char*)NULL)) { - max_size = strict_si_cast(val.c_str(), &err); + max_size = strict_iecstrtoll(val.c_str(), &err); if (!err.empty()) { cerr << "ERROR: failed to parse max size: " << err << std::endl; return EINVAL; diff --git a/src/test/objectstore_bench.cc b/src/test/objectstore_bench.cc index 406dce3a75e15..281fd522cc711 100644 --- a/src/test/objectstore_bench.cc +++ b/src/test/objectstore_bench.cc @@ -47,7 +47,7 @@ struct byte_units { bool byte_units::parse(const std::string &val, std::string *err) { - v = strict_sistrtoll(val.c_str(), err); + v = strict_iecstrtoll(val.c_str(), err); return err->empty(); } diff --git a/src/test/strtol.cc b/src/test/strtol.cc index 3946736b91572..a7ef3e51d1db7 100644 --- a/src/test/strtol.cc +++ b/src/test/strtol.cc @@ -13,6 +13,7 @@ */ #include "common/strtol.h" +#include #include #include @@ -137,6 +138,137 @@ TEST(StrToL, Error1) { } +static void test_strict_iecstrtoll(const char *str) +{ + std::string err; + strict_iecstrtoll(str, &err); + ASSERT_EQ(err, ""); +} + +static void test_strict_iecstrtoll_units(const std::string& foo, + std::string u, const int m) +{ + std::string s(foo); + s.append(u); + const char *str = s.c_str(); + std::string err; + uint64_t r = strict_iecstrtoll(str, &err); + ASSERT_EQ(err, ""); + + str = foo.c_str(); + std::string err2; + long long tmp = strict_strtoll(str, 10, &err2); + ASSERT_EQ(err2, ""); + tmp = (tmp << m); + ASSERT_EQ(tmp, (long long)r); +} + +TEST(IECStrToLL, WithUnits) { + std::map units; + units["B"] = 0; + units["K"] = 10; + units["M"] = 20; + units["G"] = 30; + units["T"] = 40; + units["P"] = 50; + units["E"] = 60; + units["Ki"] = 10; + units["Mi"] = 20; + units["Gi"] = 30; + units["Ti"] = 40; + units["Pi"] = 50; + units["Ei"] = 60; + + for (std::map::iterator p = units.begin(); + p != units.end(); ++p) { + // the upper bound of uint64_t is 2^64 = 4E + test_strict_iecstrtoll_units("4", p->first, p->second); + test_strict_iecstrtoll_units("1", p->first, p->second); + test_strict_iecstrtoll_units("0", p->first, p->second); + } +} + +TEST(IECStrToLL, WithoutUnits) { + test_strict_iecstrtoll("1024"); + test_strict_iecstrtoll("1152921504606846976"); + test_strict_iecstrtoll("0"); +} + +static void test_strict_iecstrtoll_err(const char *str) +{ + std::string err; + strict_iecstrtoll(str, &err); + ASSERT_NE(err, ""); +} + +TEST(IECStrToLL, Error) { + test_strict_iecstrtoll_err("1024F"); + test_strict_iecstrtoll_err("QDDSA"); + test_strict_iecstrtoll_err("1b"); + test_strict_iecstrtoll_err("100k"); + test_strict_iecstrtoll_err("1000m"); + test_strict_iecstrtoll_err("1g"); + test_strict_iecstrtoll_err("20t"); + test_strict_iecstrtoll_err("100p"); + test_strict_iecstrtoll_err("1000e"); + test_strict_iecstrtoll_err("B"); + test_strict_iecstrtoll_err("M"); + test_strict_iecstrtoll_err("BM"); + test_strict_iecstrtoll_err("B0wef"); + test_strict_iecstrtoll_err("0m"); + test_strict_iecstrtoll_err("-1"); // it returns uint64_t + test_strict_iecstrtoll_err("-1K"); + test_strict_iecstrtoll_err("1Bi"); + test_strict_iecstrtoll_err("Bi"); + test_strict_iecstrtoll_err("bi"); + test_strict_iecstrtoll_err("gi"); + test_strict_iecstrtoll_err("100ki"); + test_strict_iecstrtoll_err("1000mi"); + test_strict_iecstrtoll_err("1gi"); + test_strict_iecstrtoll_err("20ti"); + test_strict_iecstrtoll_err("100pi"); + test_strict_iecstrtoll_err("1000ei"); + // the upper bound of uint64_t is 2^64 = 4E, so 1024E overflows + test_strict_iecstrtoll_err("1024E"); // overflows after adding the suffix +} + +// since strict_iecstrtoll is an alias of strict_iec_cast(), quite a few +// of cases are covered by existing test cases of strict_iecstrtoll already. +TEST(StrictIECCast, Error) { + { + std::string err; + // the SI prefix is way too large for `int`. + (void)strict_iec_cast("2E", &err); + ASSERT_NE(err, ""); + } + { + std::string err; + (void)strict_iec_cast("-2E", &err); + ASSERT_NE(err, ""); + } + { + std::string err; + (void)strict_iec_cast("1T", &err); + ASSERT_NE(err, ""); + } + { + std::string err; + (void)strict_iec_cast("2E", &err); + ASSERT_EQ(err, ""); + } + { + std::string err; + (void)strict_iec_cast("-2E", &err); + ASSERT_EQ(err, ""); + } + { + std::string err; + (void)strict_iec_cast("1T", &err); + ASSERT_EQ(err, ""); + } +} + + static void test_strict_sistrtoll(const char *str) { std::string err; @@ -145,10 +277,10 @@ static void test_strict_sistrtoll(const char *str) } static void test_strict_sistrtoll_units(const std::string& foo, - char u, const int m) + std::string u, const long long m) { std::string s(foo); - s.push_back(u); + s.append(u); const char *str = s.c_str(); std::string err; uint64_t r = strict_sistrtoll(str, &err); @@ -158,21 +290,20 @@ static void test_strict_sistrtoll_units(const std::string& foo, std::string err2; long long tmp = strict_strtoll(str, 10, &err2); ASSERT_EQ(err2, ""); - tmp = (tmp << m); + tmp = (tmp * m); ASSERT_EQ(tmp, (long long)r); } TEST(SIStrToLL, WithUnits) { - std::map units; - units['B'] = 0; - units['K'] = 10; - units['M'] = 20; - units['G'] = 30; - units['T'] = 40; - units['P'] = 50; - units['E'] = 60; - - for (std::map::iterator p = units.begin(); + std::map units; + units["K"] = pow(10, 3); + units["M"] = pow(10, 6); + units["G"] = pow(10, 9); + units["T"] = pow(10, 12); + units["P"] = pow(10, 15); + units["E"] = pow(10, 18); + + for (std::map::iterator p = units.begin(); p != units.end(); ++p) { // the upper bound of uint64_t is 2^64 = 4E test_strict_sistrtoll_units("4", p->first, p->second); @@ -211,6 +342,17 @@ TEST(SIStrToLL, Error) { test_strict_sistrtoll_err("0m"); test_strict_sistrtoll_err("-1"); // it returns uint64_t test_strict_sistrtoll_err("-1K"); + test_strict_sistrtoll_err("1Bi"); + test_strict_sistrtoll_err("Bi"); + test_strict_sistrtoll_err("bi"); + test_strict_sistrtoll_err("gi"); + test_strict_sistrtoll_err("100ki"); + test_strict_sistrtoll_err("1000mi"); + test_strict_sistrtoll_err("1gi"); + test_strict_sistrtoll_err("20ti"); + test_strict_sistrtoll_err("100pi"); + test_strict_sistrtoll_err("1000ei"); + test_strict_sistrtoll_err("1B"); // the upper bound of uint64_t is 2^64 = 4E, so 1024E overflows test_strict_sistrtoll_err("1024E"); // overflows after adding the suffix } diff --git a/src/tools/rados/rados.cc b/src/tools/rados/rados.cc index 79ac8fe0fdc79..a0200eb96172d 100644 --- a/src/tools/rados/rados.cc +++ b/src/tools/rados/rados.cc @@ -248,7 +248,7 @@ unsigned default_op_size = 1 << 22; template static int rados_sistrtoll(I &i, T *val) { std::string err; - *val = strict_sistrtoll(i->second.c_str(), &err); + *val = strict_iecstrtoll(i->second.c_str(), &err); if (err != "") { cerr << "Invalid value for " << i->first << ": " << err << std::endl; return -EINVAL; diff --git a/src/tools/rbd/ArgumentTypes.cc b/src/tools/rbd/ArgumentTypes.cc index 61b84d199a952..c30a1e4bf286e 100644 --- a/src/tools/rbd/ArgumentTypes.cc +++ b/src/tools/rbd/ArgumentTypes.cc @@ -421,7 +421,7 @@ void validate(boost::any& v, const std::vector& values, const std::string &s = po::validators::get_single_string(values); std::string parse_error; - uint64_t size = strict_sistrtoll(s.c_str(), &parse_error); + uint64_t size = strict_iecstrtoll(s.c_str(), &parse_error); if (!parse_error.empty()) { throw po::validation_error(po::validation_error::invalid_option_value); } @@ -455,7 +455,7 @@ void validate(boost::any& v, const std::vector& values, const std::string &s = po::validators::get_single_string(values); std::string parse_error; - uint64_t objectsize = strict_sistrtoll(s.c_str(), &parse_error); + uint64_t objectsize = strict_iecstrtoll(s.c_str(), &parse_error); if (!parse_error.empty()) { throw po::validation_error(po::validation_error::invalid_option_value); } @@ -528,7 +528,7 @@ void validate(boost::any& v, const std::vector& values, const std::string &s = po::validators::get_single_string(values); std::string parse_error; - uint64_t size = strict_sistrtoll(s.c_str(), &parse_error); + uint64_t size = strict_iecstrtoll(s.c_str(), &parse_error); if (parse_error.empty() && (size >= (1 << 12))) { v = boost::any(size); return; @@ -542,7 +542,7 @@ void validate(boost::any& v, const std::vector& values, const std::string &s = po::validators::get_single_string(values); std::string parse_error; - uint64_t format = strict_sistrtoll(s.c_str(), &parse_error); + uint64_t format = strict_iecstrtoll(s.c_str(), &parse_error); if (!parse_error.empty() || (format != 1 && format != 2)) { throw po::validation_error(po::validation_error::invalid_option_value); } diff --git a/src/tools/rbd/action/Bench.cc b/src/tools/rbd/action/Bench.cc index 2da32bb7e6c83..0fe1950426824 100644 --- a/src/tools/rbd/action/Bench.cc +++ b/src/tools/rbd/action/Bench.cc @@ -43,7 +43,7 @@ void validate(boost::any& v, const std::vector& values, const std::string &s = po::validators::get_single_string(values); std::string parse_error; - uint64_t size = strict_sistrtoll(s.c_str(), &parse_error); + uint64_t size = strict_iecstrtoll(s.c_str(), &parse_error); if (!parse_error.empty()) { throw po::validation_error(po::validation_error::invalid_option_value); } -- 2.39.5