math.div/mod: fix div-by-0 crash

patches/command_pyroscope.cc

@@ -804,27 +804,28 @@ inline std::vector<int64_t> as_vector(const torrent::Object::list_type& args) {
 }
 
 
-int64_t apply_math_basic(const std::function<int64_t(int64_t,int64_t)> op,
+int64_t apply_math_basic(const char* name, const std::function<int64_t(int64_t,int64_t)> op,
                          const torrent::Object::list_type& args) {
-    if (args.size() == 0)
-        throw torrent::input_error("Wrong argument count in apply_math_basic.");
+    int64_t val = 0, rhs = 0;
+    bool divides = !strcmp(name, "math.div") || !strcmp(name, "math.mod");
 
-    int64_t val = 0;
+    if (args.size() == 0)
+        throw torrent::input_error(std::string(name) + ": No arguments provided!");
 
     for (torrent::Object::list_const_iterator itr = args.begin(), last = args.end(); itr != last; itr++) {
         if (itr->is_value()) {
-            val = itr == args.begin() ? itr->as_value() : op(val, itr->as_value());
+            rhs = itr->as_value();
         } else if (itr->is_string()) {
-            val = itr == args.begin()
-                ? rpc::convert_to_value(itr->as_string())
-                : op(val, rpc::convert_to_value(itr->as_string()));
+            rhs = rpc::convert_to_value(itr->as_string());
         } else if (itr->is_list()) {
-            val = itr == args.begin()
-                ? apply_math_basic(op, itr->as_list())
-                : op(val, apply_math_basic(op, itr->as_list()));
+            rhs = apply_math_basic(name, op, itr->as_list());
         } else {
-            throw torrent::input_error("Wrong type supplied to apply_math_basic.");
+            throw torrent::input_error(std::string(name) + ": Wrong argument type");
         }
+
+        if (divides && !rhs && itr != args.begin())
+            throw torrent::input_error(std::string(name) + ": Division by zero!");
+        val = itr == args.begin() ? rhs : op(val, rhs);
     }
 
     return val;
@@ -885,7 +886,7 @@ int64_t apply_arith_other(const char* op, const torrent::Object::list_type& args
         throw torrent::input_error("Wrong argument count in apply_arith_other.");
 
     if (op == "average") {
-        return (int64_t)(apply_math_basic(std::plus<int64_t>(), args) / apply_arith_count(args));
+        return (int64_t)(apply_math_basic(op, std::plus<int64_t>(), args) / apply_arith_count(args));
     } else if (op == "median") {
         std::vector<int64_t> result = as_vector(args);
         return (int64_t)rak::median(result.begin(), result.end());
@@ -960,11 +961,11 @@ void initialize_command_pyroscope() {
     CMD2_ANY("ui.focus.pgdn", _cxxstd_::bind(&cmd_ui_focus_pgdn));
     CMD2_VAR_VALUE("ui.focus.page_size", 50);
 
-    CMD2_ANY_LIST("math.add", std::bind(&apply_math_basic, std::plus<int64_t>(), std::placeholders::_2));
-    CMD2_ANY_LIST("math.sub", std::bind(&apply_math_basic, std::minus<int64_t>(), std::placeholders::_2));
-    CMD2_ANY_LIST("math.mul", std::bind(&apply_math_basic, std::multiplies<int64_t>(), std::placeholders::_2));
-    CMD2_ANY_LIST("math.div", std::bind(&apply_math_basic, std::divides<int64_t>(), std::placeholders::_2));
-    CMD2_ANY_LIST("math.mod", std::bind(&apply_math_basic, std::modulus<int64_t>(), std::placeholders::_2));
+    CMD2_ANY_LIST("math.add", std::bind(&apply_math_basic, "math.add", std::plus<int64_t>(), std::placeholders::_2));
+    CMD2_ANY_LIST("math.sub", std::bind(&apply_math_basic, "math.sub", std::minus<int64_t>(), std::placeholders::_2));
+    CMD2_ANY_LIST("math.mul", std::bind(&apply_math_basic, "math.mul", std::multiplies<int64_t>(), std::placeholders::_2));
+    CMD2_ANY_LIST("math.div", std::bind(&apply_math_basic, "math.div", std::divides<int64_t>(), std::placeholders::_2));
+    CMD2_ANY_LIST("math.mod", std::bind(&apply_math_basic, "math.mod", std::modulus<int64_t>(), std::placeholders::_2));
     CMD2_ANY_LIST("math.min", std::bind(&apply_arith_basic, std::less<int64_t>(), std::placeholders::_2));
     CMD2_ANY_LIST("math.max", std::bind(&apply_arith_basic, std::greater<int64_t>(), std::placeholders::_2));
     CMD2_ANY_LIST("math.cnt", std::bind(&apply_arith_count, std::placeholders::_2));

tasks.py

@@ -98,7 +98,7 @@ def test(ctx, name=''):
                     output = subprocess.check_output(cmd + '; echo RC=$?; exit 0',
                                                      shell=True, stderr=subprocess.STDOUT)
                     output = output.decode('utf-8')
-                elif all(x in output for x in line.split('…')):
+                elif all(x.strip() in output for x in line.split('…')):
                     print('.', end='', flush=True)
                 else:
                     failures += 1

tests/commands/math.txt

@@ -13,13 +13,26 @@ $ rtxmlrpc --repr math.add '' -- 1 +1
 2
 $ rtxmlrpc --repr math.add '' -- 1 -1
 0
+$ rtxmlrpc --repr math.add '' -- [1,2 3
+6
+$ rtxmlrpc --repr math.add '' -- text
+ERROR … Could not convert string to value
 # END
 
 # math.sub
 $ rtxmlrpc --repr math.sub '' -- 1 1
 0
 $ rtxmlrpc --repr math.sub '' -- 1 -1
 2
+$ rtxmlrpc --repr math.sub '' -- [1 [1
+0
+$ rtxmlrpc --repr math.sub '' -- [1,2
+-1
+$ rtxmlrpc --repr math.sub '' -- [9,3,3
+3
+# this breaks the law of the least surprise (it is "3 - (1 - 2)")
+$ rtxmlrpc --repr math.sub '' -- 3 [1,2
+4
 # END
 
 # math.mul
@@ -34,8 +47,12 @@ $ rtxmlrpc --repr math.div '' -- 4 +2
 2
 $ rtxmlrpc --repr math.div '' -- 8 3
 2
-#$ rtxmlrpc --repr math.div '' -- 6 0
-#2
+$ rtxmlrpc --repr math.div '' -- 0 6
+0
+$ rtxmlrpc --repr math.div '' -- 6 0
+ERROR … math.div: Division by zero
+$ rtxmlrpc --repr math.div '' --
+ERROR … math.div: No arguments provided
 # END
 
 # math.mod