wasm: fix number truncation format_int

I'm not exactly sure why we had been rounding here before. Topdown truncates when formatting a decimal number as int: format_int(15.9, 16) == "f" format_int(-15.9, 16) == "-f" Fixes #2923. Signed-off-by: Stephan Renatus <stephan.renatus@gmail.com>
open-policy-agent · Dec 14, 2020 · a2cab3f · a2cab3f
1 parent f465b22
commit a2cab3f
Show file tree

Hide file tree

Showing 3 changed files with 13 additions and 13 deletions.
diff --git a/internal/wasm/sdk/test/e2e/exceptions.yaml b/internal/wasm/sdk/test/e2e/exceptions.yaml
@@ -1,9 +1,5 @@
 # Exception Format is <test name>: <reason>
 "baseandvirtualdocs/base/virtual: conflicts": "document merge conflict - https://github.com/open-policy-agent/opa/issues/2926"
-"strings/format_int": "rounding error - https://github.com/open-policy-agent/opa/issues/2923"
-"strings/format_int: ref dest": "rounding error - https://github.com/open-policy-agent/opa/issues/2923"
-"strings/format_int: ref dest (2)": "rounding error - https://github.com/open-policy-agent/opa/issues/2923"
-"strings/format_int: undefined": "rounding error - https://github.com/open-policy-agent/opa/issues/2923"
 "withkeyword/with virtual doc specific index": "with target conflict issue - https://github.com/open-policy-agent/opa/issues/2922"
 "withkeyword/with virtual doc not specific index": "with target conflict issue - https://github.com/open-policy-agent/opa/issues/2922"
 "withkeyword/with virtual doc exact value": "with target conflict issue - https://github.com/open-policy-agent/opa/issues/2922"

diff --git a/wasm/src/strings.c b/wasm/src/strings.c
@@ -209,24 +209,26 @@ opa_value *opa_strings_format_int(opa_value *a, opa_value *b)
 
     mpd_t *i = mpd_qnew();
     uint32_t status = 0;
-
-    mpd_qround_to_intx(i, input, mpd_max_ctx(), &status);
-    if (status & ~MPD_Rounded)
+    mpd_qtrunc(i, input, mpd_max_ctx(), &status);
+    if (status != 0)
     {
-        opa_abort("strings: invalid rounding");
+        opa_abort("strings: truncate failed");
     }
 
-    opa_value *n = opa_bf_to_number(i);
-    opa_number_try_int(opa_cast_number(n), &v);
+    int32_t w = mpd_qget_i32(i, &status);
+    if (status != 0)
+    {
+        opa_abort("strings: get uint failed");
+    }
 
     char *str = opa_malloc(21); // enough for int_t (with sign).
 
-    if (v < 0)
+    if (w < 0)
     {
         str[0] = '-';
-        snprintf(&str[1], 21, format, -v);
+        snprintf(&str[1], 21, format, -w);
     } else {
-        snprintf(str, 21, format, v);
+        snprintf(str, 21, format, w);
     }
 
     return opa_string_allocated(str, opa_strlen(str));

diff --git a/wasm/tests/test.c b/wasm/tests/test.c
@@ -1796,6 +1796,8 @@ void test_strings(void)
     test("format_int/16_0", opa_value_compare(opa_strings_format_int(opa_number_float(0), opa_number_int(16)), opa_string_terminated("0")) == 0);
     test("format_int/16_1", opa_value_compare(opa_strings_format_int(opa_number_float(1), opa_number_int(16)), opa_string_terminated("1")) == 0);
     test("format_int/16_-1", opa_value_compare(opa_strings_format_int(opa_number_float(-1), opa_number_int(16)), opa_string_terminated("-1")) == 0);
+    test("format_int/16_15.5", opa_value_compare(opa_strings_format_int(opa_number_float(15.5), opa_number_int(16)), opa_string_terminated("f")) == 0);
+    test("format_int/16_-15.5", opa_value_compare(opa_strings_format_int(opa_number_float(-15.5), opa_number_int(16)), opa_string_terminated("-f")) == 0);
     test("format_int/16_16", opa_value_compare(opa_strings_format_int(opa_number_float(16), opa_number_int(16)), opa_string_terminated("10")) == 0);
     test("format_int/16_31", opa_value_compare(opa_strings_format_int(opa_number_float(31), opa_number_int(16)), opa_string_terminated("1f")) == 0);