Merge pull request ceph#58898 from neesingh-rh/wip-fix-strict-iec-cast

src/common : proper handling of units in `strict_iec_cast`

Reviewed-by: Patrick Donnelly <[email protected]>
Reviewed-by: Rishabh Dave <[email protected]>
Reviewed-by: Dhairya Parmar <[email protected]>

Author: rishabh-d-dave

qa/tasks/cephfs/test_quota.py

@@ -115,9 +115,11 @@ def test_human_readable_quota_values(self):
 
         readable_values = {"10K": "10240",
                            "100Ki": "102400",
+                           "100KiB": "102400",
                            "10M": "10485760",
                            "100Mi": "104857600",
                            "2G": "2147483648",
+                           "2GB": "2147483648",
                            "4Gi": "4294967296",
                            "1T": "1099511627776",
                            "2Ti": "2199023255552"}
@@ -135,7 +137,8 @@ def test_human_readable_quota_invalid_values(self):
 
         self.mount_a.run_shell(["mkdir", "subdir"])
 
-        invalid_values = ["10A", "1y00Ki", "af00", "G", "", " ", "-1t", "-1"]
+        invalid_values = ["10A", "1y00Ki", "af00", "G", "", " ", "-1t", "-1",
+                          "1GT", "2MM", "5Di", "8Bi", "i", "7iB"]
         for invalid_value in invalid_values:
             with self.assertRaises(CommandFailedError):
                 self.mount_a.setfattr("./subdir", "ceph.quota.max_bytes",

src/common/strtol.cc

@@ -146,43 +146,54 @@ T strict_iec_cast(std::string_view str, std::string *err)
   if (u != std::string_view::npos) {
     n = str.substr(0, u);
     unit = str.substr(u, str.length() - u);
+    // handling cases when prefixes entered as KB, MB, ...
+    // and KiB, MiB, ....
+    if (unit.length() > 1 && unit.back() == 'B') {
+      unit = unit.substr(0, unit.length() - 1);
+    }
     // we accept both old si prefixes as well as the proper iec prefixes
     // i.e. K, M, ... and Ki, Mi, ...
-    if (unit.back() == 'i') {
-      if (unit.front() == 'B') {
-        *err = "strict_iecstrtoll: illegal prefix \"Bi\"";
-        return 0;
-      }
-    }
     if (unit.length() > 2) {
       *err = "strict_iecstrtoll: illegal prefix (length > 2)";
       return 0;
     }
-    switch(unit.front()) {
-      case 'K':
-        m = 10;
-        break;
-      case 'M':
-        m = 20;
-        break;
-      case 'G':
-        m = 30;
-        break;
-      case 'T':
-        m = 40;
-        break;
-      case 'P':
-        m = 50;
-        break;
-      case 'E':
-        m = 60;
-        break;
-      case 'B':
-        break;
-      default:
-        *err = "strict_iecstrtoll: unit prefix not recognized";
-        return 0;
+    if ((unit.back() == 'i') || (unit.length() == 1)) {
+      if (unit.back() == 'i') {
+        if (unit.front() == 'B') {
+          *err = "strict_iecstrtoll: illegal prefix \"Bi\"";
+          return 0;
+        }
+      }
+      switch(unit.front()) {
+        case 'K':
+          m = 10;
+          break;
+        case 'M':
+          m = 20;
+          break;
+        case 'G':
+          m = 30;
+          break;
+        case 'T':
+          m = 40;
+          break;
+        case 'P':
+          m = 50;
+          break;
+        case 'E':
+          m = 60;
+          break;
+        case 'B':
+          break;
+        default:
+          *err = ("strict_iecstrtoll: unit prefix not recognized '" + std::string{unit} + "' ");
+          return 0;
+      }
     }
+    else {
+      *err = ("strict_iecstrtoll: illegal prefix '" + std::string{unit} + "' ");
+      return 0;
+    }   
   }
 
   long long ll = strict_strtoll(n, 10, err);

src/test/strtol.cc

@@ -184,6 +184,18 @@ TEST(IECStrToLL, WithUnits) {
   units["Ti"] = 40;
   units["Pi"] = 50;
   units["Ei"] = 60;
+  units["KB"] = 10;
+  units["MB"] = 20;
+  units["GB"] = 30;
+  units["TB"] = 40;
+  units["PB"] = 50;
+  units["EB"] = 60;
+  units["KiB"] = 10;
+  units["MiB"] = 20;
+  units["GiB"] = 30;
+  units["TiB"] = 40;
+  units["PiB"] = 50;
+  units["EiB"] = 60;
 
   for (std::map<std::string,int>::iterator p = units.begin();
        p != units.end(); ++p) {
@@ -257,6 +269,21 @@ TEST(StrictIECCast, Error) {
     (void)strict_iec_cast<int>("1T", &err);
     ASSERT_NE(err, "");
   }
+  {
+    std::string err;
+    (void)strict_iec_cast<int64_t>("1GT", &err);
+    ASSERT_NE(err, "");
+  }
+  {
+    std::string err;
+    (void)strict_iec_cast<int64_t>("1TG", &err);
+    ASSERT_NE(err, "");
+  }
+  {
+    std::string err;
+    (void)strict_iec_cast<int64_t>("1KD", &err);
+    ASSERT_NE(err, "");
+  }
   {
     std::string err;
     (void)strict_iec_cast<int64_t>("2E", &err);