Merge pull request #7390 from badone/wip-13829

common: Allow OPT_INT settings with negative values

Reviewed-by: Joao Eduardo Luis <joao@suse.de>

qa/workunits/cephtool/test.sh

@@ -1658,6 +1658,12 @@ function test_osd_bench()
   ceph tell osd.0 bench 104857600 2097152
 }
 
+function test_osd_negative_filestore_merge_threshold()
+{
+  $SUDO ceph daemon osd.0 config set filestore_merge_threshold -1
+  expect_config_value "osd.0" "filestore_merge_threshold" -1
+}
+
 function test_mon_tell()
 {
   ceph tell mon.a version
@@ -1829,6 +1835,7 @@ MON_TESTS+=" mon_ping"
 MON_TESTS+=" mon_deprecated_commands"
 MON_TESTS+=" mon_caps"
 OSD_TESTS+=" osd_bench"
+OSD_TESTS+=" osd_negative_filestore_merge_threshold"
 OSD_TESTS+=" tiering_agent"
 
 MDS_TESTS+=" mds_tell"

src/common/strtol.cc

@@ -14,10 +14,10 @@
 
 #include "strtol.h"
 
-#include <errno.h>
-#include <limits.h>
+#include <cerrno>
+#include <climits>
+#include <cstdlib>
 #include <sstream>
-#include <stdlib.h>
 
 using std::ostringstream;
 
@@ -129,14 +129,15 @@ float strict_strtof(const char *str, std::string *err)
   return ret;
 }
 
-uint64_t strict_sistrtoll(const char *str, std::string *err)
+template<typename T>
+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;
   }
-  const char &u = s.at(s.size()-1); //str[std::strlen(str)-1];
+  const char &u = s.back();
   int m = 0;
   if (u == 'B')
     m = 0;
@@ -155,30 +156,35 @@ uint64_t strict_sistrtoll(const char *str, std::string *err)
   else
     m = -1;
 
-  const char *v = NULL;
   if (m >= 0)
-    s = std::string(str, s.size()-1);
-  v = s.c_str();
+    s.pop_back();
+  else
+    m = 0;
 
-  long long r_ll = strict_strtoll(v, 10, err);
-
-  if (r_ll < 0) {
+  long long ll = strict_strtoll(s.c_str(), 10, err);
+  if (ll < 0 && !std::numeric_limits<T>::is_signed) {
     *err = "strict_sistrtoll: value should not be negative";
     return 0;
   }
-
-  uint64_t r = r_ll;
-  if (err->empty() && m > 0) {
-    if (r > (std::numeric_limits<uint64_t>::max() >> m)) {
-      *err = "strict_sistrtoll: value seems to be too large";
-      return 0;
-    }
-    r <<= m;
+  if (ll < (long long)std::numeric_limits<T>::min() >> m) {
+    *err = "strict_sistrtoll: value seems to be too small";
+    return 0;
   }
-  return r;
+  if (ll > std::numeric_limits<T>::max() >> m) {
+    *err = "strict_sistrtoll: value seems to be too large";
+    return 0;
+
+  }
+  return (ll << m);
 }
 
-template <>
-uint64_t strict_si_cast(const char *str, std::string *err) {
-  return strict_sistrtoll(str, err);
+template int strict_si_cast<int>(const char *str, std::string *err);
+
+template long long strict_si_cast<long long>(const char *str, std::string *err);
+
+template uint64_t strict_si_cast<uint64_t>(const char *str, std::string *err);
+
+uint64_t strict_sistrtoll(const char *str, std::string *err)
+{
+  return strict_si_cast<uint64_t>(str, err);
 }

src/common/strtol.h

@@ -31,21 +31,7 @@ float strict_strtof(const char *str, std::string *err);
 
 uint64_t strict_sistrtoll(const char *str, std::string *err);
 
-template <typename Target>
-Target strict_si_cast(const char *str, std::string *err) {
-  uint64_t ret = strict_sistrtoll(str, err);
-  if (!err->empty())
-    return ret;
-  if (ret > (uint64_t)std::numeric_limits<Target>::max()) {
-    err->append("The option value '");
-    err->append(str);
-    err->append("' seems to be too large");
-    return 0;
-  }
-  return ret;
-}
-
-template <>
-uint64_t strict_si_cast(const char *str, std::string *err);
+template<typename T>
+T strict_si_cast(const char *str, std::string *err);
 
 #endif

src/test/daemon_config.cc

@@ -338,14 +338,6 @@ TEST(DaemonConfig, ThreadSafety1) {
 }
 
 TEST(DaemonConfig, InvalidIntegers) {
-  {
-    int ret = g_ceph_context->_conf->set_val("num_client", "-1");
-    ASSERT_EQ(ret, -EINVAL);
-  }
-  {
-    int ret = g_ceph_context->_conf->set_val("num_client", "-1K");
-    ASSERT_EQ(ret, -EINVAL);
-  }
   {
     long long bad_value = (long long)std::numeric_limits<int>::max() + 1;
     string str = boost::lexical_cast<string>(bad_value);