[Tarantool-patches] [PATCH 0/5] Static buf in Lua, part 3

Tarantool development patches archive
 help / color / mirror / Atom feed

* [Tarantool-patches] [PATCH 0/5] Static buf in Lua, part 3
@ 2021-07-27 21:24 Vladislav Shpilevoy via Tarantool-patches
  2021-07-27 21:24 ` [Tarantool-patches] [PATCH 1/5] uuid: introduce and use luaL_pushuuidstr() Vladislav Shpilevoy via Tarantool-patches
                   ` (6 more replies)
  0 siblings, 7 replies; 20+ messages in thread
From: Vladislav Shpilevoy via Tarantool-patches @ 2021-07-27 21:24 UTC (permalink / raw)
  To: tarantool-patches, gorcunov, sergos

Here we go again.

I see that Lua UUID helpers have L suffix and decimal helpers do not. Have no
idea why, but decided not to change it here since it is not related to the patch
goal.

I validated other usages of tt_uuid_str(), decimal_str(), tt_sprintf(),
tt_statuc_buf(), tuple_str(), mp_str() and some others - they all seem to be
fine now. But you can never say for sure.

Branch: http://github.com/tarantool/tarantool/tree/gerold103/gh-6259-static_buf-part3
Issue: https://github.com/tarantool/tarantool/issues/6259

Vladislav Shpilevoy (5):
  uuid: introduce and use luaL_pushuuidstr()
  info: use luaL_pushuuidstr() for box.info uuids
  decimal: rename decimal_to_string to decimal_str
  decimal: introduce decimal_to_string
  decimal: introduce and use lua_pushdecimalstr()

 .../unreleased/gh-6259-static-buf-in-lua.md   |   5 +
 src/box/lua/info.c                            |   8 +-
 src/box/lua/net_box.c                         |   4 +-
 src/box/lua/serialize_lua.c                   |   2 +-
 src/lib/core/decimal.c                        |  10 +-
 src/lib/core/decimal.h                        |  14 +-
 src/lib/core/mp_decimal.c                     |   4 +-
 src/lua/decimal.c                             |  14 +-
 src/lua/decimal.h                             |   3 +
 src/lua/utils.c                               |  13 +
 src/lua/utils.h                               |   3 +
 ...> gh-5632-6050-6259-gc-buf-reuse.test.lua} |  85 +++-
 test/unit/decimal.c                           |   9 +-
 test/unit/decimal.result                      | 394 +++++++++---------
 third_party/lua-cjson/lua_cjson.c             |   2 +-
 third_party/lua-yaml/lyaml.cc                 |   2 +-
 16 files changed, 360 insertions(+), 212 deletions(-)
 create mode 100644 changelogs/unreleased/gh-6259-static-buf-in-lua.md
 rename test/app-tap/{gh-5632-6050-gc-buf-reuse.test.lua => gh-5632-6050-6259-gc-buf-reuse.test.lua} (68%)

-- 
2.24.3 (Apple Git-128)


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [Tarantool-patches] [PATCH 1/5] uuid: introduce and use luaL_pushuuidstr()
  2021-07-27 21:24 [Tarantool-patches] [PATCH 0/5] Static buf in Lua, part 3 Vladislav Shpilevoy via Tarantool-patches
@ 2021-07-27 21:24 ` Vladislav Shpilevoy via Tarantool-patches
  2021-07-29 11:30   ` Sergey Ostanevich via Tarantool-patches
  2021-07-27 21:24 ` [Tarantool-patches] [PATCH 2/5] info: use luaL_pushuuidstr() for box.info uuids Vladislav Shpilevoy via Tarantool-patches
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 20+ messages in thread
From: Vladislav Shpilevoy via Tarantool-patches @ 2021-07-27 21:24 UTC (permalink / raw)
  To: tarantool-patches, gorcunov, sergos

The function safely pushes tt_uuid as a string on a Lua stack.
Safety means that it does not use tt_uuid_str() which stores the
result into the global static buffer and can not be used in Lua
context.

The static buffer is not safe to use in Lua and Lua C because
during a static string push onto a Lua stack the GC might be
started and it can spoil the buffer.

Part of #6259
---
 src/box/lua/net_box.c |  4 +---
 src/lua/utils.c       | 13 +++++++++++++
 src/lua/utils.h       |  3 +++
 3 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/src/box/lua/net_box.c b/src/box/lua/net_box.c
index 3f43872ca..d1c7bedf6 100644
--- a/src/box/lua/net_box.c
+++ b/src/box/lua/net_box.c
@@ -422,7 +422,6 @@ netbox_decode_greeting(lua_State *L)
 	struct greeting greeting;
 	size_t len;
 	const char *buf = NULL;
-	char uuid_buf[UUID_STR_LEN + 1];
 
 	if (lua_isstring(L, 1))
 		buf = lua_tolstring(L, 1, &len);
@@ -443,8 +442,7 @@ netbox_decode_greeting(lua_State *L)
 	lua_pushlstring(L, greeting.salt, greeting.salt_len);
 	lua_setfield(L, -2, "salt");
 
-	tt_uuid_to_string(&greeting.uuid, uuid_buf);
-	lua_pushstring(L, uuid_buf);
+	luaL_pushuuidstr(L, &greeting.uuid);
 	lua_setfield(L, -2, "uuid");
 
 	return 1;
diff --git a/src/lua/utils.c b/src/lua/utils.c
index 34cec0eed..bcd647827 100644
--- a/src/lua/utils.c
+++ b/src/lua/utils.c
@@ -37,6 +37,7 @@
 #include <trivia/util.h>
 #include <diag.h>
 #include <fiber.h>
+#include "uuid/tt_uuid.h"
 
 int luaL_nil_ref = LUA_REFNIL;
 
@@ -107,6 +108,18 @@ luaL_pushuuid(struct lua_State *L)
 	return luaL_pushcdata(L, CTID_UUID);
 }
 
+void
+luaL_pushuuidstr(struct lua_State *L, const struct tt_uuid *uuid)
+{
+	/*
+	 * Do not use a global buffer. It might be overwritten if GC starts
+	 * working.
+	 */
+	char str[UUID_STR_LEN + 1];
+	tt_uuid_to_string(uuid, str);
+	lua_pushlstring(L, str, UUID_STR_LEN);
+}
+
 int
 luaL_iscdata(struct lua_State *L, int idx)
 {
diff --git a/src/lua/utils.h b/src/lua/utils.h
index 947d9240b..804fc9548 100644
--- a/src/lua/utils.h
+++ b/src/lua/utils.h
@@ -73,6 +73,9 @@ extern uint32_t CTID_UUID;
 struct tt_uuid *
 luaL_pushuuid(struct lua_State *L);
 
+void
+luaL_pushuuidstr(struct lua_State *L, const struct tt_uuid *uuid);
+
 /** \cond public */
 
 /**
-- 
2.24.3 (Apple Git-128)


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Tarantool-patches] [PATCH 1/5] uuid: introduce and use luaL_pushuuidstr()
  2021-07-27 21:24 ` [Tarantool-patches] [PATCH 1/5] uuid: introduce and use luaL_pushuuidstr() Vladislav Shpilevoy via Tarantool-patches
@ 2021-07-29 11:30   ` Sergey Ostanevich via Tarantool-patches
  0 siblings, 0 replies; 20+ messages in thread
From: Sergey Ostanevich via Tarantool-patches @ 2021-07-29 11:30 UTC (permalink / raw)
  To: Vladislav Shpilevoy; +Cc: tarantool-patches

Hi!

Thanks for the patch!
I believe it’s unduly to ask for a test in this particular case.

LGTM.

Sergos

> On 28 Jul 2021, at 00:24, Vladislav Shpilevoy <v.shpilevoy@tarantool.org> wrote:
> 
> The function safely pushes tt_uuid as a string on a Lua stack.
> Safety means that it does not use tt_uuid_str() which stores the
> result into the global static buffer and can not be used in Lua
> context.
> 
> The static buffer is not safe to use in Lua and Lua C because
> during a static string push onto a Lua stack the GC might be
> started and it can spoil the buffer.
> 
> Part of #6259
> ---
> src/box/lua/net_box.c |  4 +---
> src/lua/utils.c       | 13 +++++++++++++
> src/lua/utils.h       |  3 +++
> 3 files changed, 17 insertions(+), 3 deletions(-)
> 
> diff --git a/src/box/lua/net_box.c b/src/box/lua/net_box.c
> index 3f43872ca..d1c7bedf6 100644
> --- a/src/box/lua/net_box.c
> +++ b/src/box/lua/net_box.c
> @@ -422,7 +422,6 @@ netbox_decode_greeting(lua_State *L)
> 	struct greeting greeting;
> 	size_t len;
> 	const char *buf = NULL;
> -	char uuid_buf[UUID_STR_LEN + 1];
> 
> 	if (lua_isstring(L, 1))
> 		buf = lua_tolstring(L, 1, &len);
> @@ -443,8 +442,7 @@ netbox_decode_greeting(lua_State *L)
> 	lua_pushlstring(L, greeting.salt, greeting.salt_len);
> 	lua_setfield(L, -2, "salt");
> 
> -	tt_uuid_to_string(&greeting.uuid, uuid_buf);
> -	lua_pushstring(L, uuid_buf);
> +	luaL_pushuuidstr(L, &greeting.uuid);
> 	lua_setfield(L, -2, "uuid");
> 
> 	return 1;
> diff --git a/src/lua/utils.c b/src/lua/utils.c
> index 34cec0eed..bcd647827 100644
> --- a/src/lua/utils.c
> +++ b/src/lua/utils.c
> @@ -37,6 +37,7 @@
> #include <trivia/util.h>
> #include <diag.h>
> #include <fiber.h>
> +#include "uuid/tt_uuid.h"
> 
> int luaL_nil_ref = LUA_REFNIL;
> 
> @@ -107,6 +108,18 @@ luaL_pushuuid(struct lua_State *L)
> 	return luaL_pushcdata(L, CTID_UUID);
> }
> 
> +void
> +luaL_pushuuidstr(struct lua_State *L, const struct tt_uuid *uuid)
> +{
> +	/*
> +	 * Do not use a global buffer. It might be overwritten if GC starts
> +	 * working.
> +	 */
> +	char str[UUID_STR_LEN + 1];
> +	tt_uuid_to_string(uuid, str);
> +	lua_pushlstring(L, str, UUID_STR_LEN);
> +}
> +
> int
> luaL_iscdata(struct lua_State *L, int idx)
> {
> diff --git a/src/lua/utils.h b/src/lua/utils.h
> index 947d9240b..804fc9548 100644
> --- a/src/lua/utils.h
> +++ b/src/lua/utils.h
> @@ -73,6 +73,9 @@ extern uint32_t CTID_UUID;
> struct tt_uuid *
> luaL_pushuuid(struct lua_State *L);
> 
> +void
> +luaL_pushuuidstr(struct lua_State *L, const struct tt_uuid *uuid);
> +
> /** \cond public */
> 
> /**
> -- 
> 2.24.3 (Apple Git-128)
> 


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [Tarantool-patches] [PATCH 2/5] info: use luaL_pushuuidstr() for box.info uuids
  2021-07-27 21:24 [Tarantool-patches] [PATCH 0/5] Static buf in Lua, part 3 Vladislav Shpilevoy via Tarantool-patches
  2021-07-27 21:24 ` [Tarantool-patches] [PATCH 1/5] uuid: introduce and use luaL_pushuuidstr() Vladislav Shpilevoy via Tarantool-patches
@ 2021-07-27 21:24 ` Vladislav Shpilevoy via Tarantool-patches
  2021-07-29 11:38   ` Sergey Ostanevich via Tarantool-patches
  2021-07-27 21:24 ` [Tarantool-patches] [PATCH 3/5] decimal: rename decimal_to_string to decimal_str Vladislav Shpilevoy via Tarantool-patches
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 20+ messages in thread
From: Vladislav Shpilevoy via Tarantool-patches @ 2021-07-27 21:24 UTC (permalink / raw)
  To: tarantool-patches, gorcunov, sergos

box.info.uuid, box.info.cluster.uuid, and box.info.* replica UUIDs
used tt_uuid_str() function. The function is not safe to use in
preemptive context like Lua, where any attempt to push something
onto the Lua stack might trigger GC, which in turn might invoke
any other code.

It is not safe because uses the static buffer, which is global and
cyclic. Newer allocations can override the old data without any
warning.

Follow up #5632
Follow up #6050
Part of #6259
---
 src/box/lua/info.c                            |  8 ++--
 ...> gh-5632-6050-6259-gc-buf-reuse.test.lua} | 48 ++++++++++++++++---
 2 files changed, 46 insertions(+), 10 deletions(-)
 rename test/app-tap/{gh-5632-6050-gc-buf-reuse.test.lua => gh-5632-6050-6259-gc-buf-reuse.test.lua} (79%)

diff --git a/src/box/lua/info.c b/src/box/lua/info.c
index 1d8fe7938..d297ec6f6 100644
--- a/src/box/lua/info.c
+++ b/src/box/lua/info.c
@@ -176,7 +176,7 @@ lbox_pushreplica(lua_State *L, struct replica *replica)
 	lua_settable(L, -3);
 
 	lua_pushstring(L, "uuid");
-	lua_pushstring(L, tt_uuid_str(&replica->uuid));
+	luaL_pushuuidstr(L, &replica->uuid);
 	lua_settable(L, -3);
 
 	lua_pushstring(L, "lsn");
@@ -235,7 +235,7 @@ lbox_info_replication_anon_call(struct lua_State *L)
 		if (!replica->anon)
 			continue;
 
-		lua_pushstring(L, tt_uuid_str(&replica->uuid));
+		luaL_pushuuidstr(L, &replica->uuid);
 		lbox_pushreplica(L, replica);
 
 		lua_settable(L, -3);
@@ -290,7 +290,7 @@ lbox_info_id(struct lua_State *L)
 static int
 lbox_info_uuid(struct lua_State *L)
 {
-	lua_pushlstring(L, tt_uuid_str(&INSTANCE_UUID), UUID_STR_LEN);
+	luaL_pushuuidstr(L, &INSTANCE_UUID);
 	return 1;
 }
 
@@ -376,7 +376,7 @@ lbox_info_cluster(struct lua_State *L)
 {
 	lua_createtable(L, 0, 2);
 	lua_pushliteral(L, "uuid");
-	lua_pushlstring(L, tt_uuid_str(&REPLICASET_UUID), UUID_STR_LEN);
+	luaL_pushuuidstr(L, &REPLICASET_UUID);
 	lua_settable(L, -3);
 	return 1;
 }
diff --git a/test/app-tap/gh-5632-6050-gc-buf-reuse.test.lua b/test/app-tap/gh-5632-6050-6259-gc-buf-reuse.test.lua
similarity index 79%
rename from test/app-tap/gh-5632-6050-gc-buf-reuse.test.lua
rename to test/app-tap/gh-5632-6050-6259-gc-buf-reuse.test.lua
index bf7590a14..f806ba6b7 100755
--- a/test/app-tap/gh-5632-6050-gc-buf-reuse.test.lua
+++ b/test/app-tap/gh-5632-6050-6259-gc-buf-reuse.test.lua
@@ -1,10 +1,10 @@
 #!/usr/bin/env tarantool
 
 --
--- gh-5632, gh-6050: Lua code should not use any global buffers or objects
--- without proper ownership protection. Otherwise these items might be suddenly
--- reused during Lua GC which happens almost at any moment. That might lead to
--- data corruption.
+-- gh-5632, gh-6050, gh-6259: Lua code should not use any global buffers or
+-- objects without proper ownership protection. Otherwise these items might be
+-- suddenly reused during Lua GC which happens almost at any moment. That might
+-- lead to data corruption.
 --
 
 local tap = require('tap')
@@ -190,11 +190,47 @@ local function test_json(test)
    test:ok(is_success, 'json in gc')
 end
 
-local test = tap.test('gh-5632-6050-gc-buf-reuse')
-test:plan(4)
+local function test_info_uuid(test)
+    test:plan(1)
+
+    local gc_count = 100
+    local iter_count = 1000
+    local is_success = true
+
+    local function uuid_to_str()
+        local uu = uuid.new()
+        local str1 = box.info.uuid
+        local str2 = box.info.cluster.uuid
+        local str3 = box.info.uuid
+        local str4 = box.info.cluster.uuid
+        if str1 ~= str3 or str2 ~= str4 then
+            is_success = false
+            assert(false)
+        end
+    end
+
+    local function create_gc()
+        for _ = 1, gc_count do
+            ffi.gc(ffi.new('char[1]'), function() uuid_to_str() end)
+        end
+    end
+
+    for _ = 1, iter_count do
+        create_gc()
+        uuid_to_str()
+    end
+
+    test:ok(is_success, 'info uuid in gc')
+end
+
+box.cfg{}
+
+local test = tap.test('gh-5632-6050-6259-gc-buf-reuse')
+test:plan(5)
 test:test('uuid in __gc', test_uuid)
 test:test('uri in __gc', test_uri)
 test:test('msgpackffi in __gc', test_msgpackffi)
 test:test('json in __gc', test_json)
+test:test('info uuid in __gc', test_info_uuid)
 
 os.exit(test:check() and 0 or 1)
-- 
2.24.3 (Apple Git-128)


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Tarantool-patches] [PATCH 2/5] info: use luaL_pushuuidstr() for box.info uuids
  2021-07-27 21:24 ` [Tarantool-patches] [PATCH 2/5] info: use luaL_pushuuidstr() for box.info uuids Vladislav Shpilevoy via Tarantool-patches
@ 2021-07-29 11:38   ` Sergey Ostanevich via Tarantool-patches
  2021-08-01 15:03     ` Vladislav Shpilevoy via Tarantool-patches
  0 siblings, 1 reply; 20+ messages in thread
From: Sergey Ostanevich via Tarantool-patches @ 2021-07-29 11:38 UTC (permalink / raw)
  To: Vladislav Shpilevoy; +Cc: tarantool-patches

Hi!

Thanks for the patch, just one comment below.

Sergos


> On 28 Jul 2021, at 00:24, Vladislav Shpilevoy <v.shpilevoy@tarantool.org> wrote:
> 
> box.info.uuid, box.info.cluster.uuid, and box.info.* replica UUIDs
> used tt_uuid_str() function. The function is not safe to use in
> preemptive context like Lua, where any attempt to push something
> onto the Lua stack might trigger GC, which in turn might invoke
> any other code.
> 
> It is not safe because uses the static buffer, which is global and
> cyclic. Newer allocations can override the old data without any
> warning.
> 
> Follow up #5632
> Follow up #6050
> Part of #6259
> ---
> src/box/lua/info.c                            |  8 ++--
> ...> gh-5632-6050-6259-gc-buf-reuse.test.lua} | 48 ++++++++++++++++---
> 2 files changed, 46 insertions(+), 10 deletions(-)
> rename test/app-tap/{gh-5632-6050-gc-buf-reuse.test.lua => gh-5632-6050-6259-gc-buf-reuse.test.lua} (79%)
> 
> diff --git a/src/box/lua/info.c b/src/box/lua/info.c
> index 1d8fe7938..d297ec6f6 100644
> --- a/src/box/lua/info.c
> +++ b/src/box/lua/info.c
> @@ -176,7 +176,7 @@ lbox_pushreplica(lua_State *L, struct replica *replica)
> 	lua_settable(L, -3);
> 
> 	lua_pushstring(L, "uuid");
> -	lua_pushstring(L, tt_uuid_str(&replica->uuid));
> +	luaL_pushuuidstr(L, &replica->uuid);
> 	lua_settable(L, -3);
> 
> 	lua_pushstring(L, "lsn");
> @@ -235,7 +235,7 @@ lbox_info_replication_anon_call(struct lua_State *L)
> 		if (!replica->anon)
> 			continue;
> 
> -		lua_pushstring(L, tt_uuid_str(&replica->uuid));
> +		luaL_pushuuidstr(L, &replica->uuid);
> 		lbox_pushreplica(L, replica);
> 
> 		lua_settable(L, -3);
> @@ -290,7 +290,7 @@ lbox_info_id(struct lua_State *L)
> static int
> lbox_info_uuid(struct lua_State *L)
> {
> -	lua_pushlstring(L, tt_uuid_str(&INSTANCE_UUID), UUID_STR_LEN);
> +	luaL_pushuuidstr(L, &INSTANCE_UUID);
> 	return 1;
> }
> 
> @@ -376,7 +376,7 @@ lbox_info_cluster(struct lua_State *L)
> {
> 	lua_createtable(L, 0, 2);
> 	lua_pushliteral(L, "uuid");
> -	lua_pushlstring(L, tt_uuid_str(&REPLICASET_UUID), UUID_STR_LEN);
> +	luaL_pushuuidstr(L, &REPLICASET_UUID);
> 	lua_settable(L, -3);
> 	return 1;
> }
> diff --git a/test/app-tap/gh-5632-6050-gc-buf-reuse.test.lua b/test/app-tap/gh-5632-6050-6259-gc-buf-reuse.test.lua
> similarity index 79%
> rename from test/app-tap/gh-5632-6050-gc-buf-reuse.test.lua
> rename to test/app-tap/gh-5632-6050-6259-gc-buf-reuse.test.lua
> index bf7590a14..f806ba6b7 100755
> --- a/	
> +++ b/test/app-tap/gh-5632-6050-6259-gc-buf-reuse.test.lua
> @@ -1,10 +1,10 @@
> #!/usr/bin/env tarantool
> 
> --
> --- gh-5632, gh-6050: Lua code should not use any global buffers or objects
> --- without proper ownership protection. Otherwise these items might be suddenly
> --- reused during Lua GC which happens almost at any moment. That might lead to
> --- data corruption.
> +-- gh-5632, gh-6050, gh-6259: Lua code should not use any global buffers or
> +-- objects without proper ownership protection. Otherwise these items might be
> +-- suddenly reused during Lua GC which happens almost at any moment. That might
> +-- lead to data corruption.
> --
> 
> local tap = require('tap')
> @@ -190,11 +190,47 @@ local function test_json(test)
>    test:ok(is_success, 'json in gc')
> end
> 
> -local test = tap.test('gh-5632-6050-gc-buf-reuse')
> -test:plan(4)
> +local function test_info_uuid(test)
> +    test:plan(1)
> +
> +    local gc_count = 100
> +    local iter_count = 1000
> +    local is_success = true
> +
> +    local function uuid_to_str()
> +        local uu = uuid.new()
> +        local str1 = box.info.uuid
> +        local str2 = box.info.cluster.uuid
> +        local str3 = box.info.uuid
> +        local str4 = box.info.cluster.uuid
> +        if str1 ~= str3 or str2 ~= str4 then
> +            is_success = false
> +            assert(false)

Why did you keep the assert? I believe there should be just a non-ok in test?

> +        end
> +    end
> +
> +    local function create_gc()
> +        for _ = 1, gc_count do
> +            ffi.gc(ffi.new('char[1]'), function() uuid_to_str() end)
> +        end
> +    end
> +
> +    for _ = 1, iter_count do
> +        create_gc()
> +        uuid_to_str()
> +    end
> +
> +    test:ok(is_success, 'info uuid in gc')
> +end
> +
> +box.cfg{}
> +
> +local test = tap.test('gh-5632-6050-6259-gc-buf-reuse')
> +test:plan(5)
> test:test('uuid in __gc', test_uuid)
> test:test('uri in __gc', test_uri)
> test:test('msgpackffi in __gc', test_msgpackffi)
> test:test('json in __gc', test_json)
> +test:test('info uuid in __gc', test_info_uuid)
> 
> os.exit(test:check() and 0 or 1)
> -- 
> 2.24.3 (Apple Git-128)
> 


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Tarantool-patches] [PATCH 2/5] info: use luaL_pushuuidstr() for box.info uuids
  2021-07-29 11:38   ` Sergey Ostanevich via Tarantool-patches
@ 2021-08-01 15:03     ` Vladislav Shpilevoy via Tarantool-patches
  2021-08-01 17:01       ` Sergey Ostanevich via Tarantool-patches
  0 siblings, 1 reply; 20+ messages in thread
From: Vladislav Shpilevoy via Tarantool-patches @ 2021-08-01 15:03 UTC (permalink / raw)
  To: Sergey Ostanevich; +Cc: tarantool-patches

Hi! Thanks for the review!

>> diff --git a/test/app-tap/gh-5632-6050-gc-buf-reuse.test.lua b/test/app-tap/gh-5632-6050-6259-gc-buf-reuse.test.lua
>> similarity index 79%
>> rename from test/app-tap/gh-5632-6050-gc-buf-reuse.test.lua
>> rename to test/app-tap/gh-5632-6050-6259-gc-buf-reuse.test.lua
>> index bf7590a14..f806ba6b7 100755
>> --- a/	
>> +++ b/test/app-tap/gh-5632-6050-6259-gc-buf-reuse.test.lua
>> @@ -190,11 +190,47 @@ local function test_json(test)
>>    test:ok(is_success, 'json in gc')
>> end
>>
>> -local test = tap.test('gh-5632-6050-gc-buf-reuse')
>> -test:plan(4)
>> +local function test_info_uuid(test)
>> +    test:plan(1)
>> +
>> +    local gc_count = 100
>> +    local iter_count = 1000
>> +    local is_success = true
>> +
>> +    local function uuid_to_str()
>> +        local uu = uuid.new()
>> +        local str1 = box.info.uuid
>> +        local str2 = box.info.cluster.uuid
>> +        local str3 = box.info.uuid
>> +        local str4 = box.info.cluster.uuid
>> +        if str1 ~= str3 or str2 ~= str4 then
>> +            is_success = false
>> +            assert(false)
> 
> Why did you keep the assert? I believe there should be just a non-ok in test?

Dropped on the branch.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Tarantool-patches] [PATCH 2/5] info: use luaL_pushuuidstr() for box.info uuids
  2021-08-01 15:03     ` Vladislav Shpilevoy via Tarantool-patches
@ 2021-08-01 17:01       ` Sergey Ostanevich via Tarantool-patches
  0 siblings, 0 replies; 20+ messages in thread
From: Sergey Ostanevich via Tarantool-patches @ 2021-08-01 17:01 UTC (permalink / raw)
  To: Vladislav Shpilevoy; +Cc: tarantool-patches

Thanks!

LGTM.

> On 1 Aug 2021, at 18:03, Vladislav Shpilevoy <v.shpilevoy@tarantool.org> wrote:
> 
> Hi! Thanks for the review!
> 
>>> diff --git a/test/app-tap/gh-5632-6050-gc-buf-reuse.test.lua b/test/app-tap/gh-5632-6050-6259-gc-buf-reuse.test.lua
>>> similarity index 79%
>>> rename from test/app-tap/gh-5632-6050-gc-buf-reuse.test.lua
>>> rename to test/app-tap/gh-5632-6050-6259-gc-buf-reuse.test.lua
>>> index bf7590a14..f806ba6b7 100755
>>> --- a/	
>>> +++ b/test/app-tap/gh-5632-6050-6259-gc-buf-reuse.test.lua
>>> @@ -190,11 +190,47 @@ local function test_json(test)
>>>   test:ok(is_success, 'json in gc')
>>> end
>>> 
>>> -local test = tap.test('gh-5632-6050-gc-buf-reuse')
>>> -test:plan(4)
>>> +local function test_info_uuid(test)
>>> +    test:plan(1)
>>> +
>>> +    local gc_count = 100
>>> +    local iter_count = 1000
>>> +    local is_success = true
>>> +
>>> +    local function uuid_to_str()
>>> +        local uu = uuid.new()
>>> +        local str1 = box.info.uuid
>>> +        local str2 = box.info.cluster.uuid
>>> +        local str3 = box.info.uuid
>>> +        local str4 = box.info.cluster.uuid
>>> +        if str1 ~= str3 or str2 ~= str4 then
>>> +            is_success = false
>>> +            assert(false)
>> 
>> Why did you keep the assert? I believe there should be just a non-ok in test?
> 
> Dropped on the branch.


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [Tarantool-patches] [PATCH 3/5] decimal: rename decimal_to_string to decimal_str
  2021-07-27 21:24 [Tarantool-patches] [PATCH 0/5] Static buf in Lua, part 3 Vladislav Shpilevoy via Tarantool-patches
  2021-07-27 21:24 ` [Tarantool-patches] [PATCH 1/5] uuid: introduce and use luaL_pushuuidstr() Vladislav Shpilevoy via Tarantool-patches
  2021-07-27 21:24 ` [Tarantool-patches] [PATCH 2/5] info: use luaL_pushuuidstr() for box.info uuids Vladislav Shpilevoy via Tarantool-patches
@ 2021-07-27 21:24 ` Vladislav Shpilevoy via Tarantool-patches
  2021-07-29 11:41   ` Sergey Ostanevich via Tarantool-patches
  2021-07-27 21:24 ` [Tarantool-patches] [PATCH 4/5] decimal: introduce decimal_to_string Vladislav Shpilevoy via Tarantool-patches
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 20+ messages in thread
From: Vladislav Shpilevoy via Tarantool-patches @ 2021-07-27 21:24 UTC (permalink / raw)
  To: tarantool-patches, gorcunov, sergos

To be consistent with tt_uuid_str() and tt_uuid_to_string().
_str() returns a string. _to_string() copies it into an externally
passed buffer.

Part of #6259
---
 src/box/lua/serialize_lua.c       | 2 +-
 src/lib/core/decimal.c            | 2 +-
 src/lib/core/decimal.h            | 2 +-
 src/lib/core/mp_decimal.c         | 4 ++--
 src/lua/decimal.c                 | 2 +-
 test/unit/decimal.c               | 4 ++--
 third_party/lua-cjson/lua_cjson.c | 2 +-
 third_party/lua-yaml/lyaml.cc     | 2 +-
 8 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/src/box/lua/serialize_lua.c b/src/box/lua/serialize_lua.c
index 7144305cf..1f791980f 100644
--- a/src/box/lua/serialize_lua.c
+++ b/src/box/lua/serialize_lua.c
@@ -853,7 +853,7 @@ dump_node(struct lua_dumper *d, struct node *nd, int indent)
 		switch (field->ext_type) {
 		case MP_DECIMAL:
 			nd->mask |= NODE_QUOTE;
-			str = decimal_to_string(field->decval);
+			str = decimal_str(field->decval);
 			len = strlen(str);
 			break;
 		case MP_UUID:
diff --git a/src/lib/core/decimal.c b/src/lib/core/decimal.c
index 118c311d5..4befbda37 100644
--- a/src/lib/core/decimal.c
+++ b/src/lib/core/decimal.c
@@ -165,7 +165,7 @@ decimal_from_uint64(decimal_t *dec, uint64_t num)
 }
 
 const char *
-decimal_to_string(const decimal_t *dec)
+decimal_str(const decimal_t *dec)
 {
 	char *buf = tt_static_buf();
 	/* No errors are possible. */
diff --git a/src/lib/core/decimal.h b/src/lib/core/decimal.h
index 6f8762ed3..d2f2dfbdb 100644
--- a/src/lib/core/decimal.h
+++ b/src/lib/core/decimal.h
@@ -126,7 +126,7 @@ decimal_from_uint64(decimal_t *dec, uint64_t num);
  * the decimal representation.
  */
 const char *
-decimal_to_string(const decimal_t *dec);
+decimal_str(const decimal_t *dec);
 
 /**
  * Convert a given decimal to int64_t
diff --git a/src/lib/core/mp_decimal.c b/src/lib/core/mp_decimal.c
index ffc2c5773..3610b8af5 100644
--- a/src/lib/core/mp_decimal.c
+++ b/src/lib/core/mp_decimal.c
@@ -77,7 +77,7 @@ mp_snprint_decimal(char *buf, int size, const char **data, uint32_t len)
 	decimal_t d;
 	if (decimal_unpack(data, len, &d) == NULL)
 		return -1;
-	return snprintf(buf, size, "%s", decimal_to_string(&d));
+	return snprintf(buf, size, "%s", decimal_str(&d));
 }
 
 int
@@ -86,5 +86,5 @@ mp_fprint_decimal(FILE *file, const char **data, uint32_t len)
 	decimal_t d;
 	if (decimal_unpack(data, len, &d) == NULL)
 		return -1;
-	return fprintf(file, "%s", decimal_to_string(&d));
+	return fprintf(file, "%s", decimal_str(&d));
 }
diff --git a/src/lua/decimal.c b/src/lua/decimal.c
index 003680a48..e25b3ec18 100644
--- a/src/lua/decimal.c
+++ b/src/lua/decimal.c
@@ -376,7 +376,7 @@ ldecimal_tostring(struct lua_State *L)
 	if (lua_gettop(L) < 1)
 		return luaL_error(L, "usage: decimal.tostring(decimal)");
 	decimal_t *lhs = lua_checkdecimal(L, 1);
-	lua_pushstring(L, decimal_to_string(lhs));
+	lua_pushstring(L, decimal_str(lhs));
 	return 1;
 }
 
diff --git a/test/unit/decimal.c b/test/unit/decimal.c
index 32694b88a..756c68518 100644
--- a/test/unit/decimal.c
+++ b/test/unit/decimal.c
@@ -107,7 +107,7 @@ char buf[32];
 	is(b1, b2, "mp_decode(mp_encode("str") len");\
 	is(decimal_compare(&dec, &d2), 0, "mp_decode(mp_encode("str")) value");\
 	is(decimal_scale(&dec), decimal_scale(&d2), "mp_decode(mp_encode("str")) scale");\
-	is(strcmp(decimal_to_string(&d2), str), 0, "str(mp_decode(mp_encode("str"))) == "str);\
+	is(strcmp(decimal_str(&d2), str), 0, "str(mp_decode(mp_encode("str"))) == "str);\
 	b2 = buf;\
 	int8_t type;\
 	uint32_t l2 = mp_decode_extl(&b2, &type);\
@@ -131,7 +131,7 @@ char buf[32];
 	is(decimal_compare(&dec, &d2), 0, "decimal_unpack(decimal_pack("str")) value");\
 	is(decimal_scale(&dec), decimal_scale(&d2), "decimal_unpack(decimal_pack("str")) scale");\
 	is(decimal_precision(&dec), decimal_precision(&d2), "decimal_unpack(decimal_pack("str")) precision");\
-	is(strcmp(decimal_to_string(&d2), str), 0, "str(decimal_unpack(decimal_pack("str")) == "str);\
+	is(strcmp(decimal_str(&d2), str), 0, "str(decimal_unpack(decimal_pack("str")) == "str);\
 })
 
 #define test_toint(type, num, out_fmt) ({\
diff --git a/third_party/lua-cjson/lua_cjson.c b/third_party/lua-cjson/lua_cjson.c
index 5123b9a74..7a326075a 100644
--- a/third_party/lua-cjson/lua_cjson.c
+++ b/third_party/lua-cjson/lua_cjson.c
@@ -420,7 +420,7 @@ static void json_append_data(lua_State *l, struct luaL_serializer *cfg,
         switch (field.ext_type) {
         case MP_DECIMAL:
         {
-            const char *str = decimal_to_string(field.decval);
+            const char *str = decimal_str(field.decval);
             return json_append_string(cfg, json, str, strlen(str));
         }
         case MP_UUID:
diff --git a/third_party/lua-yaml/lyaml.cc b/third_party/lua-yaml/lyaml.cc
index 5469e9f4f..2b67dcc6a 100644
--- a/third_party/lua-yaml/lyaml.cc
+++ b/third_party/lua-yaml/lyaml.cc
@@ -700,7 +700,7 @@ static int dump_node(struct lua_yaml_dumper *dumper)
    case MP_EXT:
       switch (field.ext_type) {
       case MP_DECIMAL:
-         str = decimal_to_string(field.decval);
+         str = decimal_str(field.decval);
          len = strlen(str);
          break;
       case MP_UUID:
-- 
2.24.3 (Apple Git-128)


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Tarantool-patches] [PATCH 3/5] decimal: rename decimal_to_string to decimal_str
  2021-07-27 21:24 ` [Tarantool-patches] [PATCH 3/5] decimal: rename decimal_to_string to decimal_str Vladislav Shpilevoy via Tarantool-patches
@ 2021-07-29 11:41   ` Sergey Ostanevich via Tarantool-patches
  2021-08-01 15:03     ` Vladislav Shpilevoy via Tarantool-patches
  0 siblings, 1 reply; 20+ messages in thread
From: Sergey Ostanevich via Tarantool-patches @ 2021-07-29 11:41 UTC (permalink / raw)
  To: Vladislav Shpilevoy; +Cc: tarantool-patches

Hi! Thanks for the patch, just one comment re commit message.

LGTM

Sergos

> On 28 Jul 2021, at 00:24, Vladislav Shpilevoy <v.shpilevoy@tarantool.org> wrote:
> 
> To be consistent with tt_uuid_str() and tt_uuid_to_string().
> _str() returns a string. _to_string() copies it into an externally
> passed buffer.

Please, mention the decimal_to_string() function explicitly to ease the search later on.

> 
> Part of #6259
> ---
> src/box/lua/serialize_lua.c       | 2 +-
> src/lib/core/decimal.c            | 2 +-
> src/lib/core/decimal.h            | 2 +-
> src/lib/core/mp_decimal.c         | 4 ++--
> src/lua/decimal.c                 | 2 +-
> test/unit/decimal.c               | 4 ++--
> third_party/lua-cjson/lua_cjson.c | 2 +-
> third_party/lua-yaml/lyaml.cc     | 2 +-
> 8 files changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/src/box/lua/serialize_lua.c b/src/box/lua/serialize_lua.c
> index 7144305cf..1f791980f 100644
> --- a/src/box/lua/serialize_lua.c
> +++ b/src/box/lua/serialize_lua.c
> @@ -853,7 +853,7 @@ dump_node(struct lua_dumper *d, struct node *nd, int indent)
> 		switch (field->ext_type) {
> 		case MP_DECIMAL:
> 			nd->mask |= NODE_QUOTE;
> -			str = decimal_to_string(field->decval);
> +			str = decimal_str(field->decval);
> 			len = strlen(str);
> 			break;
> 		case MP_UUID:
> diff --git a/src/lib/core/decimal.c b/src/lib/core/decimal.c
> index 118c311d5..4befbda37 100644
> --- a/src/lib/core/decimal.c
> +++ b/src/lib/core/decimal.c
> @@ -165,7 +165,7 @@ decimal_from_uint64(decimal_t *dec, uint64_t num)
> }
> 
> const char *
> -decimal_to_string(const decimal_t *dec)
> +decimal_str(const decimal_t *dec)
> {
> 	char *buf = tt_static_buf();
> 	/* No errors are possible. */
> diff --git a/src/lib/core/decimal.h b/src/lib/core/decimal.h
> index 6f8762ed3..d2f2dfbdb 100644
> --- a/src/lib/core/decimal.h
> +++ b/src/lib/core/decimal.h
> @@ -126,7 +126,7 @@ decimal_from_uint64(decimal_t *dec, uint64_t num);
>  * the decimal representation.
>  */
> const char *
> -decimal_to_string(const decimal_t *dec);
> +decimal_str(const decimal_t *dec);
> 
> /**
>  * Convert a given decimal to int64_t
> diff --git a/src/lib/core/mp_decimal.c b/src/lib/core/mp_decimal.c
> index ffc2c5773..3610b8af5 100644
> --- a/src/lib/core/mp_decimal.c
> +++ b/src/lib/core/mp_decimal.c
> @@ -77,7 +77,7 @@ mp_snprint_decimal(char *buf, int size, const char **data, uint32_t len)
> 	decimal_t d;
> 	if (decimal_unpack(data, len, &d) == NULL)
> 		return -1;
> -	return snprintf(buf, size, "%s", decimal_to_string(&d));
> +	return snprintf(buf, size, "%s", decimal_str(&d));
> }
> 
> int
> @@ -86,5 +86,5 @@ mp_fprint_decimal(FILE *file, const char **data, uint32_t len)
> 	decimal_t d;
> 	if (decimal_unpack(data, len, &d) == NULL)
> 		return -1;
> -	return fprintf(file, "%s", decimal_to_string(&d));
> +	return fprintf(file, "%s", decimal_str(&d));
> }
> diff --git a/src/lua/decimal.c b/src/lua/decimal.c
> index 003680a48..e25b3ec18 100644
> --- a/src/lua/decimal.c
> +++ b/src/lua/decimal.c
> @@ -376,7 +376,7 @@ ldecimal_tostring(struct lua_State *L)
> 	if (lua_gettop(L) < 1)
> 		return luaL_error(L, "usage: decimal.tostring(decimal)");
> 	decimal_t *lhs = lua_checkdecimal(L, 1);
> -	lua_pushstring(L, decimal_to_string(lhs));
> +	lua_pushstring(L, decimal_str(lhs));
> 	return 1;
> }
> 
> diff --git a/test/unit/decimal.c b/test/unit/decimal.c
> index 32694b88a..756c68518 100644
> --- a/test/unit/decimal.c
> +++ b/test/unit/decimal.c
> @@ -107,7 +107,7 @@ char buf[32];
> 	is(b1, b2, "mp_decode(mp_encode("str") len");\
> 	is(decimal_compare(&dec, &d2), 0, "mp_decode(mp_encode("str")) value");\
> 	is(decimal_scale(&dec), decimal_scale(&d2), "mp_decode(mp_encode("str")) scale");\
> -	is(strcmp(decimal_to_string(&d2), str), 0, "str(mp_decode(mp_encode("str"))) == "str);\
> +	is(strcmp(decimal_str(&d2), str), 0, "str(mp_decode(mp_encode("str"))) == "str);\
> 	b2 = buf;\
> 	int8_t type;\
> 	uint32_t l2 = mp_decode_extl(&b2, &type);\
> @@ -131,7 +131,7 @@ char buf[32];
> 	is(decimal_compare(&dec, &d2), 0, "decimal_unpack(decimal_pack("str")) value");\
> 	is(decimal_scale(&dec), decimal_scale(&d2), "decimal_unpack(decimal_pack("str")) scale");\
> 	is(decimal_precision(&dec), decimal_precision(&d2), "decimal_unpack(decimal_pack("str")) precision");\
> -	is(strcmp(decimal_to_string(&d2), str), 0, "str(decimal_unpack(decimal_pack("str")) == "str);\
> +	is(strcmp(decimal_str(&d2), str), 0, "str(decimal_unpack(decimal_pack("str")) == "str);\
> })
> 
> #define test_toint(type, num, out_fmt) ({\
> diff --git a/third_party/lua-cjson/lua_cjson.c b/third_party/lua-cjson/lua_cjson.c
> index 5123b9a74..7a326075a 100644
> --- a/third_party/lua-cjson/lua_cjson.c
> +++ b/third_party/lua-cjson/lua_cjson.c
> @@ -420,7 +420,7 @@ static void json_append_data(lua_State *l, struct luaL_serializer *cfg,
>         switch (field.ext_type) {
>         case MP_DECIMAL:
>         {
> -            const char *str = decimal_to_string(field.decval);
> +            const char *str = decimal_str(field.decval);
>             return json_append_string(cfg, json, str, strlen(str));
>         }
>         case MP_UUID:
> diff --git a/third_party/lua-yaml/lyaml.cc b/third_party/lua-yaml/lyaml.cc
> index 5469e9f4f..2b67dcc6a 100644
> --- a/third_party/lua-yaml/lyaml.cc
> +++ b/third_party/lua-yaml/lyaml.cc
> @@ -700,7 +700,7 @@ static int dump_node(struct lua_yaml_dumper *dumper)
>    case MP_EXT:
>       switch (field.ext_type) {
>       case MP_DECIMAL:
> -         str = decimal_to_string(field.decval);
> +         str = decimal_str(field.decval);
>          len = strlen(str);
>          break;
>       case MP_UUID:
> -- 
> 2.24.3 (Apple Git-128)
> 


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Tarantool-patches] [PATCH 3/5] decimal: rename decimal_to_string to decimal_str
  2021-07-29 11:41   ` Sergey Ostanevich via Tarantool-patches
@ 2021-08-01 15:03     ` Vladislav Shpilevoy via Tarantool-patches
  2021-08-01 17:01       ` Sergey Ostanevich via Tarantool-patches
  0 siblings, 1 reply; 20+ messages in thread
From: Vladislav Shpilevoy via Tarantool-patches @ 2021-08-01 15:03 UTC (permalink / raw)
  To: Sergey Ostanevich; +Cc: tarantool-patches

Thanks for the review!

>> On 28 Jul 2021, at 00:24, Vladislav Shpilevoy <v.shpilevoy@tarantool.org> wrote:
>>
>> To be consistent with tt_uuid_str() and tt_uuid_to_string().
>> _str() returns a string. _to_string() copies it into an externally
>> passed buffer.
> 
> Please, mention the decimal_to_string() function explicitly to ease the search later on.

It is mentioned in the commit title.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Tarantool-patches] [PATCH 3/5] decimal: rename decimal_to_string to decimal_str
  2021-08-01 15:03     ` Vladislav Shpilevoy via Tarantool-patches
@ 2021-08-01 17:01       ` Sergey Ostanevich via Tarantool-patches
  0 siblings, 0 replies; 20+ messages in thread
From: Sergey Ostanevich via Tarantool-patches @ 2021-08-01 17:01 UTC (permalink / raw)
  To: Vladislav Shpilevoy; +Cc: tarantool-patches

Thanks!

LGTM.

> On 1 Aug 2021, at 18:03, Vladislav Shpilevoy <v.shpilevoy@tarantool.org> wrote:
> 
> Thanks for the review!
> 
>>> On 28 Jul 2021, at 00:24, Vladislav Shpilevoy <v.shpilevoy@tarantool.org> wrote:
>>> 
>>> To be consistent with tt_uuid_str() and tt_uuid_to_string().
>>> _str() returns a string. _to_string() copies it into an externally
>>> passed buffer.
>> 
>> Please, mention the decimal_to_string() function explicitly to ease the search later on.
> 
> It is mentioned in the commit title.


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [Tarantool-patches] [PATCH 4/5] decimal: introduce decimal_to_string
  2021-07-27 21:24 [Tarantool-patches] [PATCH 0/5] Static buf in Lua, part 3 Vladislav Shpilevoy via Tarantool-patches
                   ` (2 preceding siblings ...)
  2021-07-27 21:24 ` [Tarantool-patches] [PATCH 3/5] decimal: rename decimal_to_string to decimal_str Vladislav Shpilevoy via Tarantool-patches
@ 2021-07-27 21:24 ` Vladislav Shpilevoy via Tarantool-patches
  2021-07-29 11:52   ` Sergey Ostanevich via Tarantool-patches
  2021-07-27 21:24 ` [Tarantool-patches] [PATCH 5/5] decimal: introduce and use lua_pushdecimalstr() Vladislav Shpilevoy via Tarantool-patches
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 20+ messages in thread
From: Vladislav Shpilevoy via Tarantool-patches @ 2021-07-27 21:24 UTC (permalink / raw)
  To: tarantool-patches, gorcunov, sergos

It saves decimal as a string into an externally passed buffer.
This will be used by places which can not use the static buffer
returned by decimal_str().

Part of #6259
---
 src/lib/core/decimal.c   |   8 +
 src/lib/core/decimal.h   |  12 ++
 test/unit/decimal.c      |   5 +-
 test/unit/decimal.result | 394 ++++++++++++++++++++-------------------
 4 files changed, 230 insertions(+), 189 deletions(-)

diff --git a/src/lib/core/decimal.c b/src/lib/core/decimal.c
index 4befbda37..6d2ccb96f 100644
--- a/src/lib/core/decimal.c
+++ b/src/lib/core/decimal.c
@@ -175,6 +175,14 @@ decimal_str(const decimal_t *dec)
 	return buf;
 }
 
+void
+decimal_to_string(const decimal_t *dec, char *str)
+{
+	char *tmp = decNumberToString(dec, str);
+	assert(str == tmp);
+	(void)tmp;
+}
+
 static decimal_t *
 decimal_to_integer(decimal_t *dec)
 {
diff --git a/src/lib/core/decimal.h b/src/lib/core/decimal.h
index d2f2dfbdb..aeafd2c68 100644
--- a/src/lib/core/decimal.h
+++ b/src/lib/core/decimal.h
@@ -42,6 +42,14 @@
 extern "C" {
 #endif /* defined(__cplusplus) */
 
+enum {
+	/*
+	 * The documentation in the decnumber sources says the string needs to
+	 * be >= digit count + 14.
+	 */
+	DECIMAL_MAX_STR_LEN = DECIMAL_MAX_DIGITS + 14,
+};
+
 typedef decNumber decimal_t;
 
 /**
@@ -128,6 +136,10 @@ decimal_from_uint64(decimal_t *dec, uint64_t num);
 const char *
 decimal_str(const decimal_t *dec);
 
+/** Write the decimal as a string into the passed buffer. */
+void
+decimal_to_string(const decimal_t *dec, char *str);
+
 /**
  * Convert a given decimal to int64_t
  * \param[out] num - the result
diff --git a/test/unit/decimal.c b/test/unit/decimal.c
index 756c68518..aea646e15 100644
--- a/test/unit/decimal.c
+++ b/test/unit/decimal.c
@@ -108,6 +108,9 @@ char buf[32];
 	is(decimal_compare(&dec, &d2), 0, "mp_decode(mp_encode("str")) value");\
 	is(decimal_scale(&dec), decimal_scale(&d2), "mp_decode(mp_encode("str")) scale");\
 	is(strcmp(decimal_str(&d2), str), 0, "str(mp_decode(mp_encode("str"))) == "str);\
+	char strbuf[DECIMAL_MAX_STR_LEN + 1];\
+	decimal_to_string(&dec, strbuf);\
+	is(strcmp(strbuf, decimal_str(&dec)), 0, "stack str == static str for "str);\
 	b2 = buf;\
 	int8_t type;\
 	uint32_t l2 = mp_decode_extl(&b2, &type);\
@@ -194,7 +197,7 @@ test_pack_unpack(void)
 static int
 test_mp_decimal(void)
 {
-	plan(198);
+	plan(216);
 
 	test_mpdec("0");
 	test_mpdec("-0");
diff --git a/test/unit/decimal.result b/test/unit/decimal.result
index 8f6de61a7..b7da2d2ce 100644
--- a/test/unit/decimal.result
+++ b/test/unit/decimal.result
@@ -498,7 +498,7 @@ ok 279 - subtests
     ok 150 - unpack malformed decimal fails
     ok 151 - decode malformed decimal preserves buffer position
 ok 280 - subtests
-    1..198
+    1..216
     ok 1 - mp_sizeof_decimal(0)
     ok 2 - mp_sizeof_decimal(0) == len(mp_encode_decimal(0))
     ok 3 - mp_next(mp_encode(0))
@@ -506,197 +506,215 @@ ok 280 - subtests
     ok 5 - mp_decode(mp_encode(0)) value
     ok 6 - mp_decode(mp_encode(0)) scale
     ok 7 - str(mp_decode(mp_encode(0))) == 0
-    ok 8 - mp_ext_type is MP_DECIMAL
-    ok 9 - decimal_unpack() after mp_decode_extl()
-    ok 10 - decimal_unpack() after mp_decode_extl() value
-    ok 11 - decimal_unpack() after mp_decode_extl() len
-    ok 12 - mp_sizeof_decimal(-0)
-    ok 13 - mp_sizeof_decimal(-0) == len(mp_encode_decimal(-0))
-    ok 14 - mp_next(mp_encode(-0))
-    ok 15 - mp_decode(mp_encode(-0) len
-    ok 16 - mp_decode(mp_encode(-0)) value
-    ok 17 - mp_decode(mp_encode(-0)) scale
-    ok 18 - str(mp_decode(mp_encode(-0))) == -0
-    ok 19 - mp_ext_type is MP_DECIMAL
-    ok 20 - decimal_unpack() after mp_decode_extl()
-    ok 21 - decimal_unpack() after mp_decode_extl() value
-    ok 22 - decimal_unpack() after mp_decode_extl() len
-    ok 23 - mp_sizeof_decimal(1)
-    ok 24 - mp_sizeof_decimal(1) == len(mp_encode_decimal(1))
-    ok 25 - mp_next(mp_encode(1))
-    ok 26 - mp_decode(mp_encode(1) len
-    ok 27 - mp_decode(mp_encode(1)) value
-    ok 28 - mp_decode(mp_encode(1)) scale
-    ok 29 - str(mp_decode(mp_encode(1))) == 1
-    ok 30 - mp_ext_type is MP_DECIMAL
-    ok 31 - decimal_unpack() after mp_decode_extl()
-    ok 32 - decimal_unpack() after mp_decode_extl() value
-    ok 33 - decimal_unpack() after mp_decode_extl() len
-    ok 34 - mp_sizeof_decimal(-1)
-    ok 35 - mp_sizeof_decimal(-1) == len(mp_encode_decimal(-1))
-    ok 36 - mp_next(mp_encode(-1))
-    ok 37 - mp_decode(mp_encode(-1) len
-    ok 38 - mp_decode(mp_encode(-1)) value
-    ok 39 - mp_decode(mp_encode(-1)) scale
-    ok 40 - str(mp_decode(mp_encode(-1))) == -1
-    ok 41 - mp_ext_type is MP_DECIMAL
-    ok 42 - decimal_unpack() after mp_decode_extl()
-    ok 43 - decimal_unpack() after mp_decode_extl() value
-    ok 44 - decimal_unpack() after mp_decode_extl() len
-    ok 45 - mp_sizeof_decimal(0.1)
-    ok 46 - mp_sizeof_decimal(0.1) == len(mp_encode_decimal(0.1))
-    ok 47 - mp_next(mp_encode(0.1))
-    ok 48 - mp_decode(mp_encode(0.1) len
-    ok 49 - mp_decode(mp_encode(0.1)) value
-    ok 50 - mp_decode(mp_encode(0.1)) scale
-    ok 51 - str(mp_decode(mp_encode(0.1))) == 0.1
-    ok 52 - mp_ext_type is MP_DECIMAL
-    ok 53 - decimal_unpack() after mp_decode_extl()
-    ok 54 - decimal_unpack() after mp_decode_extl() value
-    ok 55 - decimal_unpack() after mp_decode_extl() len
-    ok 56 - mp_sizeof_decimal(-0.1)
-    ok 57 - mp_sizeof_decimal(-0.1) == len(mp_encode_decimal(-0.1))
-    ok 58 - mp_next(mp_encode(-0.1))
-    ok 59 - mp_decode(mp_encode(-0.1) len
-    ok 60 - mp_decode(mp_encode(-0.1)) value
-    ok 61 - mp_decode(mp_encode(-0.1)) scale
-    ok 62 - str(mp_decode(mp_encode(-0.1))) == -0.1
-    ok 63 - mp_ext_type is MP_DECIMAL
-    ok 64 - decimal_unpack() after mp_decode_extl()
-    ok 65 - decimal_unpack() after mp_decode_extl() value
-    ok 66 - decimal_unpack() after mp_decode_extl() len
-    ok 67 - mp_sizeof_decimal(2.718281828459045)
-    ok 68 - mp_sizeof_decimal(2.718281828459045) == len(mp_encode_decimal(2.718281828459045))
-    ok 69 - mp_next(mp_encode(2.718281828459045))
-    ok 70 - mp_decode(mp_encode(2.718281828459045) len
-    ok 71 - mp_decode(mp_encode(2.718281828459045)) value
-    ok 72 - mp_decode(mp_encode(2.718281828459045)) scale
-    ok 73 - str(mp_decode(mp_encode(2.718281828459045))) == 2.718281828459045
-    ok 74 - mp_ext_type is MP_DECIMAL
-    ok 75 - decimal_unpack() after mp_decode_extl()
-    ok 76 - decimal_unpack() after mp_decode_extl() value
-    ok 77 - decimal_unpack() after mp_decode_extl() len
-    ok 78 - mp_sizeof_decimal(-2.718281828459045)
-    ok 79 - mp_sizeof_decimal(-2.718281828459045) == len(mp_encode_decimal(-2.718281828459045))
-    ok 80 - mp_next(mp_encode(-2.718281828459045))
-    ok 81 - mp_decode(mp_encode(-2.718281828459045) len
-    ok 82 - mp_decode(mp_encode(-2.718281828459045)) value
-    ok 83 - mp_decode(mp_encode(-2.718281828459045)) scale
-    ok 84 - str(mp_decode(mp_encode(-2.718281828459045))) == -2.718281828459045
-    ok 85 - mp_ext_type is MP_DECIMAL
-    ok 86 - decimal_unpack() after mp_decode_extl()
-    ok 87 - decimal_unpack() after mp_decode_extl() value
-    ok 88 - decimal_unpack() after mp_decode_extl() len
-    ok 89 - mp_sizeof_decimal(3.141592653589793)
-    ok 90 - mp_sizeof_decimal(3.141592653589793) == len(mp_encode_decimal(3.141592653589793))
-    ok 91 - mp_next(mp_encode(3.141592653589793))
-    ok 92 - mp_decode(mp_encode(3.141592653589793) len
-    ok 93 - mp_decode(mp_encode(3.141592653589793)) value
-    ok 94 - mp_decode(mp_encode(3.141592653589793)) scale
-    ok 95 - str(mp_decode(mp_encode(3.141592653589793))) == 3.141592653589793
-    ok 96 - mp_ext_type is MP_DECIMAL
-    ok 97 - decimal_unpack() after mp_decode_extl()
-    ok 98 - decimal_unpack() after mp_decode_extl() value
-    ok 99 - decimal_unpack() after mp_decode_extl() len
-    ok 100 - mp_sizeof_decimal(-3.141592653589793)
-    ok 101 - mp_sizeof_decimal(-3.141592653589793) == len(mp_encode_decimal(-3.141592653589793))
-    ok 102 - mp_next(mp_encode(-3.141592653589793))
-    ok 103 - mp_decode(mp_encode(-3.141592653589793) len
-    ok 104 - mp_decode(mp_encode(-3.141592653589793)) value
-    ok 105 - mp_decode(mp_encode(-3.141592653589793)) scale
-    ok 106 - str(mp_decode(mp_encode(-3.141592653589793))) == -3.141592653589793
-    ok 107 - mp_ext_type is MP_DECIMAL
-    ok 108 - decimal_unpack() after mp_decode_extl()
-    ok 109 - decimal_unpack() after mp_decode_extl() value
-    ok 110 - decimal_unpack() after mp_decode_extl() len
-    ok 111 - mp_sizeof_decimal(1234567891234567890.0987654321987654321)
-    ok 112 - mp_sizeof_decimal(1234567891234567890.0987654321987654321) == len(mp_encode_decimal(1234567891234567890.0987654321987654321))
-    ok 113 - mp_next(mp_encode(1234567891234567890.0987654321987654321))
-    ok 114 - mp_decode(mp_encode(1234567891234567890.0987654321987654321) len
-    ok 115 - mp_decode(mp_encode(1234567891234567890.0987654321987654321)) value
-    ok 116 - mp_decode(mp_encode(1234567891234567890.0987654321987654321)) scale
-    ok 117 - str(mp_decode(mp_encode(1234567891234567890.0987654321987654321))) == 1234567891234567890.0987654321987654321
-    ok 118 - mp_ext_type is MP_DECIMAL
-    ok 119 - decimal_unpack() after mp_decode_extl()
-    ok 120 - decimal_unpack() after mp_decode_extl() value
-    ok 121 - decimal_unpack() after mp_decode_extl() len
-    ok 122 - mp_sizeof_decimal(-1234567891234567890.0987654321987654321)
-    ok 123 - mp_sizeof_decimal(-1234567891234567890.0987654321987654321) == len(mp_encode_decimal(-1234567891234567890.0987654321987654321))
-    ok 124 - mp_next(mp_encode(-1234567891234567890.0987654321987654321))
-    ok 125 - mp_decode(mp_encode(-1234567891234567890.0987654321987654321) len
-    ok 126 - mp_decode(mp_encode(-1234567891234567890.0987654321987654321)) value
-    ok 127 - mp_decode(mp_encode(-1234567891234567890.0987654321987654321)) scale
-    ok 128 - str(mp_decode(mp_encode(-1234567891234567890.0987654321987654321))) == -1234567891234567890.0987654321987654321
+    ok 8 - stack str == static str for 0
+    ok 9 - mp_ext_type is MP_DECIMAL
+    ok 10 - decimal_unpack() after mp_decode_extl()
+    ok 11 - decimal_unpack() after mp_decode_extl() value
+    ok 12 - decimal_unpack() after mp_decode_extl() len
+    ok 13 - mp_sizeof_decimal(-0)
+    ok 14 - mp_sizeof_decimal(-0) == len(mp_encode_decimal(-0))
+    ok 15 - mp_next(mp_encode(-0))
+    ok 16 - mp_decode(mp_encode(-0) len
+    ok 17 - mp_decode(mp_encode(-0)) value
+    ok 18 - mp_decode(mp_encode(-0)) scale
+    ok 19 - str(mp_decode(mp_encode(-0))) == -0
+    ok 20 - stack str == static str for -0
+    ok 21 - mp_ext_type is MP_DECIMAL
+    ok 22 - decimal_unpack() after mp_decode_extl()
+    ok 23 - decimal_unpack() after mp_decode_extl() value
+    ok 24 - decimal_unpack() after mp_decode_extl() len
+    ok 25 - mp_sizeof_decimal(1)
+    ok 26 - mp_sizeof_decimal(1) == len(mp_encode_decimal(1))
+    ok 27 - mp_next(mp_encode(1))
+    ok 28 - mp_decode(mp_encode(1) len
+    ok 29 - mp_decode(mp_encode(1)) value
+    ok 30 - mp_decode(mp_encode(1)) scale
+    ok 31 - str(mp_decode(mp_encode(1))) == 1
+    ok 32 - stack str == static str for 1
+    ok 33 - mp_ext_type is MP_DECIMAL
+    ok 34 - decimal_unpack() after mp_decode_extl()
+    ok 35 - decimal_unpack() after mp_decode_extl() value
+    ok 36 - decimal_unpack() after mp_decode_extl() len
+    ok 37 - mp_sizeof_decimal(-1)
+    ok 38 - mp_sizeof_decimal(-1) == len(mp_encode_decimal(-1))
+    ok 39 - mp_next(mp_encode(-1))
+    ok 40 - mp_decode(mp_encode(-1) len
+    ok 41 - mp_decode(mp_encode(-1)) value
+    ok 42 - mp_decode(mp_encode(-1)) scale
+    ok 43 - str(mp_decode(mp_encode(-1))) == -1
+    ok 44 - stack str == static str for -1
+    ok 45 - mp_ext_type is MP_DECIMAL
+    ok 46 - decimal_unpack() after mp_decode_extl()
+    ok 47 - decimal_unpack() after mp_decode_extl() value
+    ok 48 - decimal_unpack() after mp_decode_extl() len
+    ok 49 - mp_sizeof_decimal(0.1)
+    ok 50 - mp_sizeof_decimal(0.1) == len(mp_encode_decimal(0.1))
+    ok 51 - mp_next(mp_encode(0.1))
+    ok 52 - mp_decode(mp_encode(0.1) len
+    ok 53 - mp_decode(mp_encode(0.1)) value
+    ok 54 - mp_decode(mp_encode(0.1)) scale
+    ok 55 - str(mp_decode(mp_encode(0.1))) == 0.1
+    ok 56 - stack str == static str for 0.1
+    ok 57 - mp_ext_type is MP_DECIMAL
+    ok 58 - decimal_unpack() after mp_decode_extl()
+    ok 59 - decimal_unpack() after mp_decode_extl() value
+    ok 60 - decimal_unpack() after mp_decode_extl() len
+    ok 61 - mp_sizeof_decimal(-0.1)
+    ok 62 - mp_sizeof_decimal(-0.1) == len(mp_encode_decimal(-0.1))
+    ok 63 - mp_next(mp_encode(-0.1))
+    ok 64 - mp_decode(mp_encode(-0.1) len
+    ok 65 - mp_decode(mp_encode(-0.1)) value
+    ok 66 - mp_decode(mp_encode(-0.1)) scale
+    ok 67 - str(mp_decode(mp_encode(-0.1))) == -0.1
+    ok 68 - stack str == static str for -0.1
+    ok 69 - mp_ext_type is MP_DECIMAL
+    ok 70 - decimal_unpack() after mp_decode_extl()
+    ok 71 - decimal_unpack() after mp_decode_extl() value
+    ok 72 - decimal_unpack() after mp_decode_extl() len
+    ok 73 - mp_sizeof_decimal(2.718281828459045)
+    ok 74 - mp_sizeof_decimal(2.718281828459045) == len(mp_encode_decimal(2.718281828459045))
+    ok 75 - mp_next(mp_encode(2.718281828459045))
+    ok 76 - mp_decode(mp_encode(2.718281828459045) len
+    ok 77 - mp_decode(mp_encode(2.718281828459045)) value
+    ok 78 - mp_decode(mp_encode(2.718281828459045)) scale
+    ok 79 - str(mp_decode(mp_encode(2.718281828459045))) == 2.718281828459045
+    ok 80 - stack str == static str for 2.718281828459045
+    ok 81 - mp_ext_type is MP_DECIMAL
+    ok 82 - decimal_unpack() after mp_decode_extl()
+    ok 83 - decimal_unpack() after mp_decode_extl() value
+    ok 84 - decimal_unpack() after mp_decode_extl() len
+    ok 85 - mp_sizeof_decimal(-2.718281828459045)
+    ok 86 - mp_sizeof_decimal(-2.718281828459045) == len(mp_encode_decimal(-2.718281828459045))
+    ok 87 - mp_next(mp_encode(-2.718281828459045))
+    ok 88 - mp_decode(mp_encode(-2.718281828459045) len
+    ok 89 - mp_decode(mp_encode(-2.718281828459045)) value
+    ok 90 - mp_decode(mp_encode(-2.718281828459045)) scale
+    ok 91 - str(mp_decode(mp_encode(-2.718281828459045))) == -2.718281828459045
+    ok 92 - stack str == static str for -2.718281828459045
+    ok 93 - mp_ext_type is MP_DECIMAL
+    ok 94 - decimal_unpack() after mp_decode_extl()
+    ok 95 - decimal_unpack() after mp_decode_extl() value
+    ok 96 - decimal_unpack() after mp_decode_extl() len
+    ok 97 - mp_sizeof_decimal(3.141592653589793)
+    ok 98 - mp_sizeof_decimal(3.141592653589793) == len(mp_encode_decimal(3.141592653589793))
+    ok 99 - mp_next(mp_encode(3.141592653589793))
+    ok 100 - mp_decode(mp_encode(3.141592653589793) len
+    ok 101 - mp_decode(mp_encode(3.141592653589793)) value
+    ok 102 - mp_decode(mp_encode(3.141592653589793)) scale
+    ok 103 - str(mp_decode(mp_encode(3.141592653589793))) == 3.141592653589793
+    ok 104 - stack str == static str for 3.141592653589793
+    ok 105 - mp_ext_type is MP_DECIMAL
+    ok 106 - decimal_unpack() after mp_decode_extl()
+    ok 107 - decimal_unpack() after mp_decode_extl() value
+    ok 108 - decimal_unpack() after mp_decode_extl() len
+    ok 109 - mp_sizeof_decimal(-3.141592653589793)
+    ok 110 - mp_sizeof_decimal(-3.141592653589793) == len(mp_encode_decimal(-3.141592653589793))
+    ok 111 - mp_next(mp_encode(-3.141592653589793))
+    ok 112 - mp_decode(mp_encode(-3.141592653589793) len
+    ok 113 - mp_decode(mp_encode(-3.141592653589793)) value
+    ok 114 - mp_decode(mp_encode(-3.141592653589793)) scale
+    ok 115 - str(mp_decode(mp_encode(-3.141592653589793))) == -3.141592653589793
+    ok 116 - stack str == static str for -3.141592653589793
+    ok 117 - mp_ext_type is MP_DECIMAL
+    ok 118 - decimal_unpack() after mp_decode_extl()
+    ok 119 - decimal_unpack() after mp_decode_extl() value
+    ok 120 - decimal_unpack() after mp_decode_extl() len
+    ok 121 - mp_sizeof_decimal(1234567891234567890.0987654321987654321)
+    ok 122 - mp_sizeof_decimal(1234567891234567890.0987654321987654321) == len(mp_encode_decimal(1234567891234567890.0987654321987654321))
+    ok 123 - mp_next(mp_encode(1234567891234567890.0987654321987654321))
+    ok 124 - mp_decode(mp_encode(1234567891234567890.0987654321987654321) len
+    ok 125 - mp_decode(mp_encode(1234567891234567890.0987654321987654321)) value
+    ok 126 - mp_decode(mp_encode(1234567891234567890.0987654321987654321)) scale
+    ok 127 - str(mp_decode(mp_encode(1234567891234567890.0987654321987654321))) == 1234567891234567890.0987654321987654321
+    ok 128 - stack str == static str for 1234567891234567890.0987654321987654321
     ok 129 - mp_ext_type is MP_DECIMAL
     ok 130 - decimal_unpack() after mp_decode_extl()
     ok 131 - decimal_unpack() after mp_decode_extl() value
     ok 132 - decimal_unpack() after mp_decode_extl() len
-    ok 133 - mp_sizeof_decimal(0.0000000000000000000000000000000000001)
-    ok 134 - mp_sizeof_decimal(0.0000000000000000000000000000000000001) == len(mp_encode_decimal(0.0000000000000000000000000000000000001))
-    ok 135 - mp_next(mp_encode(0.0000000000000000000000000000000000001))
-    ok 136 - mp_decode(mp_encode(0.0000000000000000000000000000000000001) len
-    ok 137 - mp_decode(mp_encode(0.0000000000000000000000000000000000001)) value
-    ok 138 - mp_decode(mp_encode(0.0000000000000000000000000000000000001)) scale
-    ok 139 - str(mp_decode(mp_encode(0.0000000000000000000000000000000000001))) == 0.0000000000000000000000000000000000001
-    ok 140 - mp_ext_type is MP_DECIMAL
-    ok 141 - decimal_unpack() after mp_decode_extl()
-    ok 142 - decimal_unpack() after mp_decode_extl() value
-    ok 143 - decimal_unpack() after mp_decode_extl() len
-    ok 144 - mp_sizeof_decimal(-0.0000000000000000000000000000000000001)
-    ok 145 - mp_sizeof_decimal(-0.0000000000000000000000000000000000001) == len(mp_encode_decimal(-0.0000000000000000000000000000000000001))
-    ok 146 - mp_next(mp_encode(-0.0000000000000000000000000000000000001))
-    ok 147 - mp_decode(mp_encode(-0.0000000000000000000000000000000000001) len
-    ok 148 - mp_decode(mp_encode(-0.0000000000000000000000000000000000001)) value
-    ok 149 - mp_decode(mp_encode(-0.0000000000000000000000000000000000001)) scale
-    ok 150 - str(mp_decode(mp_encode(-0.0000000000000000000000000000000000001))) == -0.0000000000000000000000000000000000001
-    ok 151 - mp_ext_type is MP_DECIMAL
-    ok 152 - decimal_unpack() after mp_decode_extl()
-    ok 153 - decimal_unpack() after mp_decode_extl() value
-    ok 154 - decimal_unpack() after mp_decode_extl() len
-    ok 155 - mp_sizeof_decimal(0.00000000000000000000000000000000000001)
-    ok 156 - mp_sizeof_decimal(0.00000000000000000000000000000000000001) == len(mp_encode_decimal(0.00000000000000000000000000000000000001))
-    ok 157 - mp_next(mp_encode(0.00000000000000000000000000000000000001))
-    ok 158 - mp_decode(mp_encode(0.00000000000000000000000000000000000001) len
-    ok 159 - mp_decode(mp_encode(0.00000000000000000000000000000000000001)) value
-    ok 160 - mp_decode(mp_encode(0.00000000000000000000000000000000000001)) scale
-    ok 161 - str(mp_decode(mp_encode(0.00000000000000000000000000000000000001))) == 0.00000000000000000000000000000000000001
-    ok 162 - mp_ext_type is MP_DECIMAL
-    ok 163 - decimal_unpack() after mp_decode_extl()
-    ok 164 - decimal_unpack() after mp_decode_extl() value
-    ok 165 - decimal_unpack() after mp_decode_extl() len
-    ok 166 - mp_sizeof_decimal(-0.00000000000000000000000000000000000001)
-    ok 167 - mp_sizeof_decimal(-0.00000000000000000000000000000000000001) == len(mp_encode_decimal(-0.00000000000000000000000000000000000001))
-    ok 168 - mp_next(mp_encode(-0.00000000000000000000000000000000000001))
-    ok 169 - mp_decode(mp_encode(-0.00000000000000000000000000000000000001) len
-    ok 170 - mp_decode(mp_encode(-0.00000000000000000000000000000000000001)) value
-    ok 171 - mp_decode(mp_encode(-0.00000000000000000000000000000000000001)) scale
-    ok 172 - str(mp_decode(mp_encode(-0.00000000000000000000000000000000000001))) == -0.00000000000000000000000000000000000001
-    ok 173 - mp_ext_type is MP_DECIMAL
-    ok 174 - decimal_unpack() after mp_decode_extl()
-    ok 175 - decimal_unpack() after mp_decode_extl() value
-    ok 176 - decimal_unpack() after mp_decode_extl() len
-    ok 177 - mp_sizeof_decimal(99999999999999999999999999999999999999)
-    ok 178 - mp_sizeof_decimal(99999999999999999999999999999999999999) == len(mp_encode_decimal(99999999999999999999999999999999999999))
-    ok 179 - mp_next(mp_encode(99999999999999999999999999999999999999))
-    ok 180 - mp_decode(mp_encode(99999999999999999999999999999999999999) len
-    ok 181 - mp_decode(mp_encode(99999999999999999999999999999999999999)) value
-    ok 182 - mp_decode(mp_encode(99999999999999999999999999999999999999)) scale
-    ok 183 - str(mp_decode(mp_encode(99999999999999999999999999999999999999))) == 99999999999999999999999999999999999999
-    ok 184 - mp_ext_type is MP_DECIMAL
-    ok 185 - decimal_unpack() after mp_decode_extl()
-    ok 186 - decimal_unpack() after mp_decode_extl() value
-    ok 187 - decimal_unpack() after mp_decode_extl() len
-    ok 188 - mp_sizeof_decimal(-99999999999999999999999999999999999999)
-    ok 189 - mp_sizeof_decimal(-99999999999999999999999999999999999999) == len(mp_encode_decimal(-99999999999999999999999999999999999999))
-    ok 190 - mp_next(mp_encode(-99999999999999999999999999999999999999))
-    ok 191 - mp_decode(mp_encode(-99999999999999999999999999999999999999) len
-    ok 192 - mp_decode(mp_encode(-99999999999999999999999999999999999999)) value
-    ok 193 - mp_decode(mp_encode(-99999999999999999999999999999999999999)) scale
-    ok 194 - str(mp_decode(mp_encode(-99999999999999999999999999999999999999))) == -99999999999999999999999999999999999999
-    ok 195 - mp_ext_type is MP_DECIMAL
-    ok 196 - decimal_unpack() after mp_decode_extl()
-    ok 197 - decimal_unpack() after mp_decode_extl() value
-    ok 198 - decimal_unpack() after mp_decode_extl() len
+    ok 133 - mp_sizeof_decimal(-1234567891234567890.0987654321987654321)
+    ok 134 - mp_sizeof_decimal(-1234567891234567890.0987654321987654321) == len(mp_encode_decimal(-1234567891234567890.0987654321987654321))
+    ok 135 - mp_next(mp_encode(-1234567891234567890.0987654321987654321))
+    ok 136 - mp_decode(mp_encode(-1234567891234567890.0987654321987654321) len
+    ok 137 - mp_decode(mp_encode(-1234567891234567890.0987654321987654321)) value
+    ok 138 - mp_decode(mp_encode(-1234567891234567890.0987654321987654321)) scale
+    ok 139 - str(mp_decode(mp_encode(-1234567891234567890.0987654321987654321))) == -1234567891234567890.0987654321987654321
+    ok 140 - stack str == static str for -1234567891234567890.0987654321987654321
+    ok 141 - mp_ext_type is MP_DECIMAL
+    ok 142 - decimal_unpack() after mp_decode_extl()
+    ok 143 - decimal_unpack() after mp_decode_extl() value
+    ok 144 - decimal_unpack() after mp_decode_extl() len
+    ok 145 - mp_sizeof_decimal(0.0000000000000000000000000000000000001)
+    ok 146 - mp_sizeof_decimal(0.0000000000000000000000000000000000001) == len(mp_encode_decimal(0.0000000000000000000000000000000000001))
+    ok 147 - mp_next(mp_encode(0.0000000000000000000000000000000000001))
+    ok 148 - mp_decode(mp_encode(0.0000000000000000000000000000000000001) len
+    ok 149 - mp_decode(mp_encode(0.0000000000000000000000000000000000001)) value
+    ok 150 - mp_decode(mp_encode(0.0000000000000000000000000000000000001)) scale
+    ok 151 - str(mp_decode(mp_encode(0.0000000000000000000000000000000000001))) == 0.0000000000000000000000000000000000001
+    ok 152 - stack str == static str for 0.0000000000000000000000000000000000001
+    ok 153 - mp_ext_type is MP_DECIMAL
+    ok 154 - decimal_unpack() after mp_decode_extl()
+    ok 155 - decimal_unpack() after mp_decode_extl() value
+    ok 156 - decimal_unpack() after mp_decode_extl() len
+    ok 157 - mp_sizeof_decimal(-0.0000000000000000000000000000000000001)
+    ok 158 - mp_sizeof_decimal(-0.0000000000000000000000000000000000001) == len(mp_encode_decimal(-0.0000000000000000000000000000000000001))
+    ok 159 - mp_next(mp_encode(-0.0000000000000000000000000000000000001))
+    ok 160 - mp_decode(mp_encode(-0.0000000000000000000000000000000000001) len
+    ok 161 - mp_decode(mp_encode(-0.0000000000000000000000000000000000001)) value
+    ok 162 - mp_decode(mp_encode(-0.0000000000000000000000000000000000001)) scale
+    ok 163 - str(mp_decode(mp_encode(-0.0000000000000000000000000000000000001))) == -0.0000000000000000000000000000000000001
+    ok 164 - stack str == static str for -0.0000000000000000000000000000000000001
+    ok 165 - mp_ext_type is MP_DECIMAL
+    ok 166 - decimal_unpack() after mp_decode_extl()
+    ok 167 - decimal_unpack() after mp_decode_extl() value
+    ok 168 - decimal_unpack() after mp_decode_extl() len
+    ok 169 - mp_sizeof_decimal(0.00000000000000000000000000000000000001)
+    ok 170 - mp_sizeof_decimal(0.00000000000000000000000000000000000001) == len(mp_encode_decimal(0.00000000000000000000000000000000000001))
+    ok 171 - mp_next(mp_encode(0.00000000000000000000000000000000000001))
+    ok 172 - mp_decode(mp_encode(0.00000000000000000000000000000000000001) len
+    ok 173 - mp_decode(mp_encode(0.00000000000000000000000000000000000001)) value
+    ok 174 - mp_decode(mp_encode(0.00000000000000000000000000000000000001)) scale
+    ok 175 - str(mp_decode(mp_encode(0.00000000000000000000000000000000000001))) == 0.00000000000000000000000000000000000001
+    ok 176 - stack str == static str for 0.00000000000000000000000000000000000001
+    ok 177 - mp_ext_type is MP_DECIMAL
+    ok 178 - decimal_unpack() after mp_decode_extl()
+    ok 179 - decimal_unpack() after mp_decode_extl() value
+    ok 180 - decimal_unpack() after mp_decode_extl() len
+    ok 181 - mp_sizeof_decimal(-0.00000000000000000000000000000000000001)
+    ok 182 - mp_sizeof_decimal(-0.00000000000000000000000000000000000001) == len(mp_encode_decimal(-0.00000000000000000000000000000000000001))
+    ok 183 - mp_next(mp_encode(-0.00000000000000000000000000000000000001))
+    ok 184 - mp_decode(mp_encode(-0.00000000000000000000000000000000000001) len
+    ok 185 - mp_decode(mp_encode(-0.00000000000000000000000000000000000001)) value
+    ok 186 - mp_decode(mp_encode(-0.00000000000000000000000000000000000001)) scale
+    ok 187 - str(mp_decode(mp_encode(-0.00000000000000000000000000000000000001))) == -0.00000000000000000000000000000000000001
+    ok 188 - stack str == static str for -0.00000000000000000000000000000000000001
+    ok 189 - mp_ext_type is MP_DECIMAL
+    ok 190 - decimal_unpack() after mp_decode_extl()
+    ok 191 - decimal_unpack() after mp_decode_extl() value
+    ok 192 - decimal_unpack() after mp_decode_extl() len
+    ok 193 - mp_sizeof_decimal(99999999999999999999999999999999999999)
+    ok 194 - mp_sizeof_decimal(99999999999999999999999999999999999999) == len(mp_encode_decimal(99999999999999999999999999999999999999))
+    ok 195 - mp_next(mp_encode(99999999999999999999999999999999999999))
+    ok 196 - mp_decode(mp_encode(99999999999999999999999999999999999999) len
+    ok 197 - mp_decode(mp_encode(99999999999999999999999999999999999999)) value
+    ok 198 - mp_decode(mp_encode(99999999999999999999999999999999999999)) scale
+    ok 199 - str(mp_decode(mp_encode(99999999999999999999999999999999999999))) == 99999999999999999999999999999999999999
+    ok 200 - stack str == static str for 99999999999999999999999999999999999999
+    ok 201 - mp_ext_type is MP_DECIMAL
+    ok 202 - decimal_unpack() after mp_decode_extl()
+    ok 203 - decimal_unpack() after mp_decode_extl() value
+    ok 204 - decimal_unpack() after mp_decode_extl() len
+    ok 205 - mp_sizeof_decimal(-99999999999999999999999999999999999999)
+    ok 206 - mp_sizeof_decimal(-99999999999999999999999999999999999999) == len(mp_encode_decimal(-99999999999999999999999999999999999999))
+    ok 207 - mp_next(mp_encode(-99999999999999999999999999999999999999))
+    ok 208 - mp_decode(mp_encode(-99999999999999999999999999999999999999) len
+    ok 209 - mp_decode(mp_encode(-99999999999999999999999999999999999999)) value
+    ok 210 - mp_decode(mp_encode(-99999999999999999999999999999999999999)) scale
+    ok 211 - str(mp_decode(mp_encode(-99999999999999999999999999999999999999))) == -99999999999999999999999999999999999999
+    ok 212 - stack str == static str for -99999999999999999999999999999999999999
+    ok 213 - mp_ext_type is MP_DECIMAL
+    ok 214 - decimal_unpack() after mp_decode_extl()
+    ok 215 - decimal_unpack() after mp_decode_extl() value
+    ok 216 - decimal_unpack() after mp_decode_extl() len
 ok 281 - subtests
     1..5
 	*** test_mp_print ***
-- 
2.24.3 (Apple Git-128)


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Tarantool-patches] [PATCH 4/5] decimal: introduce decimal_to_string
  2021-07-27 21:24 ` [Tarantool-patches] [PATCH 4/5] decimal: introduce decimal_to_string Vladislav Shpilevoy via Tarantool-patches
@ 2021-07-29 11:52   ` Sergey Ostanevich via Tarantool-patches
  2021-08-01 15:04     ` Vladislav Shpilevoy via Tarantool-patches
  0 siblings, 1 reply; 20+ messages in thread
From: Sergey Ostanevich via Tarantool-patches @ 2021-07-29 11:52 UTC (permalink / raw)
  To: Vladislav Shpilevoy; +Cc: tarantool-patches

Hi! Thanks for the patch, my 2 cents below. 

Otherwise - LGTM.

Regards,
Sergos


> On 28 Jul 2021, at 00:24, Vladislav Shpilevoy <v.shpilevoy@tarantool.org> wrote:
> 
> It saves decimal as a string into an externally passed buffer.

Please mention decimal_to_string() introduction.

> This will be used by places which can not use the static buffer
> returned by decimal_str().
> 
> Part of #6259
> ---
> src/lib/core/decimal.c   |   8 +
> src/lib/core/decimal.h   |  12 ++
> test/unit/decimal.c      |   5 +-
> test/unit/decimal.result | 394 ++++++++++++++++++++-------------------
> 4 files changed, 230 insertions(+), 189 deletions(-)
> 
> diff --git a/src/lib/core/decimal.c b/src/lib/core/decimal.c
> index 4befbda37..6d2ccb96f 100644
> --- a/src/lib/core/decimal.c
> +++ b/src/lib/core/decimal.c
> @@ -175,6 +175,14 @@ decimal_str(const decimal_t *dec)
> 	return buf;
> }
> 
> +void
> +decimal_to_string(const decimal_t *dec, char *str)
> +{
> +	char *tmp = decNumberToString(dec, str);
> +	assert(str == tmp);

Why? The decToString() doesn't touch original string pointer, working on
a local copy.

> +	(void)tmp;
> +}
> +
> static decimal_t *
> decimal_to_integer(decimal_t *dec)
> {
> diff --git a/src/lib/core/decimal.h b/src/lib/core/decimal.h
> index d2f2dfbdb..aeafd2c68 100644
> --- a/src/lib/core/decimal.h
> +++ b/src/lib/core/decimal.h
> @@ -42,6 +42,14 @@
> extern "C" {
> #endif /* defined(__cplusplus) */
> 
> +enum {
> +	/*
> +	 * The documentation in the decnumber sources says the string needs to
> +	 * be >= digit count + 14.
> +	 */
> +	DECIMAL_MAX_STR_LEN = DECIMAL_MAX_DIGITS + 14,
> +};
> +
> typedef decNumber decimal_t;
> 
> /**
> @@ -128,6 +136,10 @@ decimal_from_uint64(decimal_t *dec, uint64_t num);
> const char *
> decimal_str(const decimal_t *dec);
> 
> +/** Write the decimal as a string into the passed buffer. */
> +void
> +decimal_to_string(const decimal_t *dec, char *str);
> +
> /**
>  * Convert a given decimal to int64_t
>  * \param[out] num - the result
> diff --git a/test/unit/decimal.c b/test/unit/decimal.c
> index 756c68518..aea646e15 100644
> --- a/test/unit/decimal.c
> +++ b/test/unit/decimal.c
> @@ -108,6 +108,9 @@ char buf[32];
> 	is(decimal_compare(&dec, &d2), 0, "mp_decode(mp_encode("str")) value");\
> 	is(decimal_scale(&dec), decimal_scale(&d2), "mp_decode(mp_encode("str")) scale");\
> 	is(strcmp(decimal_str(&d2), str), 0, "str(mp_decode(mp_encode("str"))) == "str);\
> +	char strbuf[DECIMAL_MAX_STR_LEN + 1];\
> +	decimal_to_string(&dec, strbuf);\
> +	is(strcmp(strbuf, decimal_str(&dec)), 0, "stack str == static str for "str);\
> 	b2 = buf;\
> 	int8_t type;\
> 	uint32_t l2 = mp_decode_extl(&b2, &type);\
> @@ -194,7 +197,7 @@ test_pack_unpack(void)
> static int
> test_mp_decimal(void)
> {
> -	plan(198);
> +	plan(216);
> 
> 	test_mpdec("0");
> 	test_mpdec("-0");
> diff --git a/test/unit/decimal.result b/test/unit/decimal.result
> index 8f6de61a7..b7da2d2ce 100644
> --- a/test/unit/decimal.result
> +++ b/test/unit/decimal.result
> @@ -498,7 +498,7 @@ ok 279 - subtests
>     ok 150 - unpack malformed decimal fails
>     ok 151 - decode malformed decimal preserves buffer position
> ok 280 - subtests
> -    1..198
> +    1..216
>     ok 1 - mp_sizeof_decimal(0)
>     ok 2 - mp_sizeof_decimal(0) == len(mp_encode_decimal(0))
>     ok 3 - mp_next(mp_encode(0))
> @@ -506,197 +506,215 @@ ok 280 - subtests
>     ok 5 - mp_decode(mp_encode(0)) value
>     ok 6 - mp_decode(mp_encode(0)) scale
>     ok 7 - str(mp_decode(mp_encode(0))) == 0
> -    ok 8 - mp_ext_type is MP_DECIMAL
> -    ok 9 - decimal_unpack() after mp_decode_extl()
> -    ok 10 - decimal_unpack() after mp_decode_extl() value
> -    ok 11 - decimal_unpack() after mp_decode_extl() len
> -    ok 12 - mp_sizeof_decimal(-0)
> -    ok 13 - mp_sizeof_decimal(-0) == len(mp_encode_decimal(-0))
> -    ok 14 - mp_next(mp_encode(-0))
> -    ok 15 - mp_decode(mp_encode(-0) len
> -    ok 16 - mp_decode(mp_encode(-0)) value
> -    ok 17 - mp_decode(mp_encode(-0)) scale
> -    ok 18 - str(mp_decode(mp_encode(-0))) == -0
> -    ok 19 - mp_ext_type is MP_DECIMAL
> -    ok 20 - decimal_unpack() after mp_decode_extl()
> -    ok 21 - decimal_unpack() after mp_decode_extl() value
> -    ok 22 - decimal_unpack() after mp_decode_extl() len
> -    ok 23 - mp_sizeof_decimal(1)
> -    ok 24 - mp_sizeof_decimal(1) == len(mp_encode_decimal(1))
> -    ok 25 - mp_next(mp_encode(1))
> -    ok 26 - mp_decode(mp_encode(1) len
> -    ok 27 - mp_decode(mp_encode(1)) value
> -    ok 28 - mp_decode(mp_encode(1)) scale
> -    ok 29 - str(mp_decode(mp_encode(1))) == 1
> -    ok 30 - mp_ext_type is MP_DECIMAL
> -    ok 31 - decimal_unpack() after mp_decode_extl()
> -    ok 32 - decimal_unpack() after mp_decode_extl() value
> -    ok 33 - decimal_unpack() after mp_decode_extl() len
> -    ok 34 - mp_sizeof_decimal(-1)
> -    ok 35 - mp_sizeof_decimal(-1) == len(mp_encode_decimal(-1))
> -    ok 36 - mp_next(mp_encode(-1))
> -    ok 37 - mp_decode(mp_encode(-1) len
> -    ok 38 - mp_decode(mp_encode(-1)) value
> -    ok 39 - mp_decode(mp_encode(-1)) scale
> -    ok 40 - str(mp_decode(mp_encode(-1))) == -1
> -    ok 41 - mp_ext_type is MP_DECIMAL
> -    ok 42 - decimal_unpack() after mp_decode_extl()
> -    ok 43 - decimal_unpack() after mp_decode_extl() value
> -    ok 44 - decimal_unpack() after mp_decode_extl() len
> -    ok 45 - mp_sizeof_decimal(0.1)
> -    ok 46 - mp_sizeof_decimal(0.1) == len(mp_encode_decimal(0.1))
> -    ok 47 - mp_next(mp_encode(0.1))
> -    ok 48 - mp_decode(mp_encode(0.1) len
> -    ok 49 - mp_decode(mp_encode(0.1)) value
> -    ok 50 - mp_decode(mp_encode(0.1)) scale
> -    ok 51 - str(mp_decode(mp_encode(0.1))) == 0.1
> -    ok 52 - mp_ext_type is MP_DECIMAL
> -    ok 53 - decimal_unpack() after mp_decode_extl()
> -    ok 54 - decimal_unpack() after mp_decode_extl() value
> -    ok 55 - decimal_unpack() after mp_decode_extl() len
> -    ok 56 - mp_sizeof_decimal(-0.1)
> -    ok 57 - mp_sizeof_decimal(-0.1) == len(mp_encode_decimal(-0.1))
> -    ok 58 - mp_next(mp_encode(-0.1))
> -    ok 59 - mp_decode(mp_encode(-0.1) len
> -    ok 60 - mp_decode(mp_encode(-0.1)) value
> -    ok 61 - mp_decode(mp_encode(-0.1)) scale
> -    ok 62 - str(mp_decode(mp_encode(-0.1))) == -0.1
> -    ok 63 - mp_ext_type is MP_DECIMAL
> -    ok 64 - decimal_unpack() after mp_decode_extl()
> -    ok 65 - decimal_unpack() after mp_decode_extl() value
> -    ok 66 - decimal_unpack() after mp_decode_extl() len
> -    ok 67 - mp_sizeof_decimal(2.718281828459045)
> -    ok 68 - mp_sizeof_decimal(2.718281828459045) == len(mp_encode_decimal(2.718281828459045))
> -    ok 69 - mp_next(mp_encode(2.718281828459045))
> -    ok 70 - mp_decode(mp_encode(2.718281828459045) len
> -    ok 71 - mp_decode(mp_encode(2.718281828459045)) value
> -    ok 72 - mp_decode(mp_encode(2.718281828459045)) scale
> -    ok 73 - str(mp_decode(mp_encode(2.718281828459045))) == 2.718281828459045
> -    ok 74 - mp_ext_type is MP_DECIMAL
> -    ok 75 - decimal_unpack() after mp_decode_extl()
> -    ok 76 - decimal_unpack() after mp_decode_extl() value
> -    ok 77 - decimal_unpack() after mp_decode_extl() len
> -    ok 78 - mp_sizeof_decimal(-2.718281828459045)
> -    ok 79 - mp_sizeof_decimal(-2.718281828459045) == len(mp_encode_decimal(-2.718281828459045))
> -    ok 80 - mp_next(mp_encode(-2.718281828459045))
> -    ok 81 - mp_decode(mp_encode(-2.718281828459045) len
> -    ok 82 - mp_decode(mp_encode(-2.718281828459045)) value
> -    ok 83 - mp_decode(mp_encode(-2.718281828459045)) scale
> -    ok 84 - str(mp_decode(mp_encode(-2.718281828459045))) == -2.718281828459045
> -    ok 85 - mp_ext_type is MP_DECIMAL
> -    ok 86 - decimal_unpack() after mp_decode_extl()
> -    ok 87 - decimal_unpack() after mp_decode_extl() value
> -    ok 88 - decimal_unpack() after mp_decode_extl() len
> -    ok 89 - mp_sizeof_decimal(3.141592653589793)
> -    ok 90 - mp_sizeof_decimal(3.141592653589793) == len(mp_encode_decimal(3.141592653589793))
> -    ok 91 - mp_next(mp_encode(3.141592653589793))
> -    ok 92 - mp_decode(mp_encode(3.141592653589793) len
> -    ok 93 - mp_decode(mp_encode(3.141592653589793)) value
> -    ok 94 - mp_decode(mp_encode(3.141592653589793)) scale
> -    ok 95 - str(mp_decode(mp_encode(3.141592653589793))) == 3.141592653589793
> -    ok 96 - mp_ext_type is MP_DECIMAL
> -    ok 97 - decimal_unpack() after mp_decode_extl()
> -    ok 98 - decimal_unpack() after mp_decode_extl() value
> -    ok 99 - decimal_unpack() after mp_decode_extl() len
> -    ok 100 - mp_sizeof_decimal(-3.141592653589793)
> -    ok 101 - mp_sizeof_decimal(-3.141592653589793) == len(mp_encode_decimal(-3.141592653589793))
> -    ok 102 - mp_next(mp_encode(-3.141592653589793))
> -    ok 103 - mp_decode(mp_encode(-3.141592653589793) len
> -    ok 104 - mp_decode(mp_encode(-3.141592653589793)) value
> -    ok 105 - mp_decode(mp_encode(-3.141592653589793)) scale
> -    ok 106 - str(mp_decode(mp_encode(-3.141592653589793))) == -3.141592653589793
> -    ok 107 - mp_ext_type is MP_DECIMAL
> -    ok 108 - decimal_unpack() after mp_decode_extl()
> -    ok 109 - decimal_unpack() after mp_decode_extl() value
> -    ok 110 - decimal_unpack() after mp_decode_extl() len
> -    ok 111 - mp_sizeof_decimal(1234567891234567890.0987654321987654321)
> -    ok 112 - mp_sizeof_decimal(1234567891234567890.0987654321987654321) == len(mp_encode_decimal(1234567891234567890.0987654321987654321))
> -    ok 113 - mp_next(mp_encode(1234567891234567890.0987654321987654321))
> -    ok 114 - mp_decode(mp_encode(1234567891234567890.0987654321987654321) len
> -    ok 115 - mp_decode(mp_encode(1234567891234567890.0987654321987654321)) value
> -    ok 116 - mp_decode(mp_encode(1234567891234567890.0987654321987654321)) scale
> -    ok 117 - str(mp_decode(mp_encode(1234567891234567890.0987654321987654321))) == 1234567891234567890.0987654321987654321
> -    ok 118 - mp_ext_type is MP_DECIMAL
> -    ok 119 - decimal_unpack() after mp_decode_extl()
> -    ok 120 - decimal_unpack() after mp_decode_extl() value
> -    ok 121 - decimal_unpack() after mp_decode_extl() len
> -    ok 122 - mp_sizeof_decimal(-1234567891234567890.0987654321987654321)
> -    ok 123 - mp_sizeof_decimal(-1234567891234567890.0987654321987654321) == len(mp_encode_decimal(-1234567891234567890.0987654321987654321))
> -    ok 124 - mp_next(mp_encode(-1234567891234567890.0987654321987654321))
> -    ok 125 - mp_decode(mp_encode(-1234567891234567890.0987654321987654321) len
> -    ok 126 - mp_decode(mp_encode(-1234567891234567890.0987654321987654321)) value
> -    ok 127 - mp_decode(mp_encode(-1234567891234567890.0987654321987654321)) scale
> -    ok 128 - str(mp_decode(mp_encode(-1234567891234567890.0987654321987654321))) == -1234567891234567890.0987654321987654321
> +    ok 8 - stack str == static str for 0
> +    ok 9 - mp_ext_type is MP_DECIMAL
> +    ok 10 - decimal_unpack() after mp_decode_extl()
> +    ok 11 - decimal_unpack() after mp_decode_extl() value
> +    ok 12 - decimal_unpack() after mp_decode_extl() len
> +    ok 13 - mp_sizeof_decimal(-0)
> +    ok 14 - mp_sizeof_decimal(-0) == len(mp_encode_decimal(-0))
> +    ok 15 - mp_next(mp_encode(-0))
> +    ok 16 - mp_decode(mp_encode(-0) len
> +    ok 17 - mp_decode(mp_encode(-0)) value
> +    ok 18 - mp_decode(mp_encode(-0)) scale
> +    ok 19 - str(mp_decode(mp_encode(-0))) == -0
> +    ok 20 - stack str == static str for -0
> +    ok 21 - mp_ext_type is MP_DECIMAL
> +    ok 22 - decimal_unpack() after mp_decode_extl()
> +    ok 23 - decimal_unpack() after mp_decode_extl() value
> +    ok 24 - decimal_unpack() after mp_decode_extl() len
> +    ok 25 - mp_sizeof_decimal(1)
> +    ok 26 - mp_sizeof_decimal(1) == len(mp_encode_decimal(1))
> +    ok 27 - mp_next(mp_encode(1))
> +    ok 28 - mp_decode(mp_encode(1) len
> +    ok 29 - mp_decode(mp_encode(1)) value
> +    ok 30 - mp_decode(mp_encode(1)) scale
> +    ok 31 - str(mp_decode(mp_encode(1))) == 1
> +    ok 32 - stack str == static str for 1
> +    ok 33 - mp_ext_type is MP_DECIMAL
> +    ok 34 - decimal_unpack() after mp_decode_extl()
> +    ok 35 - decimal_unpack() after mp_decode_extl() value
> +    ok 36 - decimal_unpack() after mp_decode_extl() len
> +    ok 37 - mp_sizeof_decimal(-1)
> +    ok 38 - mp_sizeof_decimal(-1) == len(mp_encode_decimal(-1))
> +    ok 39 - mp_next(mp_encode(-1))
> +    ok 40 - mp_decode(mp_encode(-1) len
> +    ok 41 - mp_decode(mp_encode(-1)) value
> +    ok 42 - mp_decode(mp_encode(-1)) scale
> +    ok 43 - str(mp_decode(mp_encode(-1))) == -1
> +    ok 44 - stack str == static str for -1
> +    ok 45 - mp_ext_type is MP_DECIMAL
> +    ok 46 - decimal_unpack() after mp_decode_extl()
> +    ok 47 - decimal_unpack() after mp_decode_extl() value
> +    ok 48 - decimal_unpack() after mp_decode_extl() len
> +    ok 49 - mp_sizeof_decimal(0.1)
> +    ok 50 - mp_sizeof_decimal(0.1) == len(mp_encode_decimal(0.1))
> +    ok 51 - mp_next(mp_encode(0.1))
> +    ok 52 - mp_decode(mp_encode(0.1) len
> +    ok 53 - mp_decode(mp_encode(0.1)) value
> +    ok 54 - mp_decode(mp_encode(0.1)) scale
> +    ok 55 - str(mp_decode(mp_encode(0.1))) == 0.1
> +    ok 56 - stack str == static str for 0.1
> +    ok 57 - mp_ext_type is MP_DECIMAL
> +    ok 58 - decimal_unpack() after mp_decode_extl()
> +    ok 59 - decimal_unpack() after mp_decode_extl() value
> +    ok 60 - decimal_unpack() after mp_decode_extl() len
> +    ok 61 - mp_sizeof_decimal(-0.1)
> +    ok 62 - mp_sizeof_decimal(-0.1) == len(mp_encode_decimal(-0.1))
> +    ok 63 - mp_next(mp_encode(-0.1))
> +    ok 64 - mp_decode(mp_encode(-0.1) len
> +    ok 65 - mp_decode(mp_encode(-0.1)) value
> +    ok 66 - mp_decode(mp_encode(-0.1)) scale
> +    ok 67 - str(mp_decode(mp_encode(-0.1))) == -0.1
> +    ok 68 - stack str == static str for -0.1
> +    ok 69 - mp_ext_type is MP_DECIMAL
> +    ok 70 - decimal_unpack() after mp_decode_extl()
> +    ok 71 - decimal_unpack() after mp_decode_extl() value
> +    ok 72 - decimal_unpack() after mp_decode_extl() len
> +    ok 73 - mp_sizeof_decimal(2.718281828459045)
> +    ok 74 - mp_sizeof_decimal(2.718281828459045) == len(mp_encode_decimal(2.718281828459045))
> +    ok 75 - mp_next(mp_encode(2.718281828459045))
> +    ok 76 - mp_decode(mp_encode(2.718281828459045) len
> +    ok 77 - mp_decode(mp_encode(2.718281828459045)) value
> +    ok 78 - mp_decode(mp_encode(2.718281828459045)) scale
> +    ok 79 - str(mp_decode(mp_encode(2.718281828459045))) == 2.718281828459045
> +    ok 80 - stack str == static str for 2.718281828459045
> +    ok 81 - mp_ext_type is MP_DECIMAL
> +    ok 82 - decimal_unpack() after mp_decode_extl()
> +    ok 83 - decimal_unpack() after mp_decode_extl() value
> +    ok 84 - decimal_unpack() after mp_decode_extl() len
> +    ok 85 - mp_sizeof_decimal(-2.718281828459045)
> +    ok 86 - mp_sizeof_decimal(-2.718281828459045) == len(mp_encode_decimal(-2.718281828459045))
> +    ok 87 - mp_next(mp_encode(-2.718281828459045))
> +    ok 88 - mp_decode(mp_encode(-2.718281828459045) len
> +    ok 89 - mp_decode(mp_encode(-2.718281828459045)) value
> +    ok 90 - mp_decode(mp_encode(-2.718281828459045)) scale
> +    ok 91 - str(mp_decode(mp_encode(-2.718281828459045))) == -2.718281828459045
> +    ok 92 - stack str == static str for -2.718281828459045
> +    ok 93 - mp_ext_type is MP_DECIMAL
> +    ok 94 - decimal_unpack() after mp_decode_extl()
> +    ok 95 - decimal_unpack() after mp_decode_extl() value
> +    ok 96 - decimal_unpack() after mp_decode_extl() len
> +    ok 97 - mp_sizeof_decimal(3.141592653589793)
> +    ok 98 - mp_sizeof_decimal(3.141592653589793) == len(mp_encode_decimal(3.141592653589793))
> +    ok 99 - mp_next(mp_encode(3.141592653589793))
> +    ok 100 - mp_decode(mp_encode(3.141592653589793) len
> +    ok 101 - mp_decode(mp_encode(3.141592653589793)) value
> +    ok 102 - mp_decode(mp_encode(3.141592653589793)) scale
> +    ok 103 - str(mp_decode(mp_encode(3.141592653589793))) == 3.141592653589793
> +    ok 104 - stack str == static str for 3.141592653589793
> +    ok 105 - mp_ext_type is MP_DECIMAL
> +    ok 106 - decimal_unpack() after mp_decode_extl()
> +    ok 107 - decimal_unpack() after mp_decode_extl() value
> +    ok 108 - decimal_unpack() after mp_decode_extl() len
> +    ok 109 - mp_sizeof_decimal(-3.141592653589793)
> +    ok 110 - mp_sizeof_decimal(-3.141592653589793) == len(mp_encode_decimal(-3.141592653589793))
> +    ok 111 - mp_next(mp_encode(-3.141592653589793))
> +    ok 112 - mp_decode(mp_encode(-3.141592653589793) len
> +    ok 113 - mp_decode(mp_encode(-3.141592653589793)) value
> +    ok 114 - mp_decode(mp_encode(-3.141592653589793)) scale
> +    ok 115 - str(mp_decode(mp_encode(-3.141592653589793))) == -3.141592653589793
> +    ok 116 - stack str == static str for -3.141592653589793
> +    ok 117 - mp_ext_type is MP_DECIMAL
> +    ok 118 - decimal_unpack() after mp_decode_extl()
> +    ok 119 - decimal_unpack() after mp_decode_extl() value
> +    ok 120 - decimal_unpack() after mp_decode_extl() len
> +    ok 121 - mp_sizeof_decimal(1234567891234567890.0987654321987654321)
> +    ok 122 - mp_sizeof_decimal(1234567891234567890.0987654321987654321) == len(mp_encode_decimal(1234567891234567890.0987654321987654321))
> +    ok 123 - mp_next(mp_encode(1234567891234567890.0987654321987654321))
> +    ok 124 - mp_decode(mp_encode(1234567891234567890.0987654321987654321) len
> +    ok 125 - mp_decode(mp_encode(1234567891234567890.0987654321987654321)) value
> +    ok 126 - mp_decode(mp_encode(1234567891234567890.0987654321987654321)) scale
> +    ok 127 - str(mp_decode(mp_encode(1234567891234567890.0987654321987654321))) == 1234567891234567890.0987654321987654321
> +    ok 128 - stack str == static str for 1234567891234567890.0987654321987654321
>     ok 129 - mp_ext_type is MP_DECIMAL
>     ok 130 - decimal_unpack() after mp_decode_extl()
>     ok 131 - decimal_unpack() after mp_decode_extl() value
>     ok 132 - decimal_unpack() after mp_decode_extl() len
> -    ok 133 - mp_sizeof_decimal(0.0000000000000000000000000000000000001)
> -    ok 134 - mp_sizeof_decimal(0.0000000000000000000000000000000000001) == len(mp_encode_decimal(0.0000000000000000000000000000000000001))
> -    ok 135 - mp_next(mp_encode(0.0000000000000000000000000000000000001))
> -    ok 136 - mp_decode(mp_encode(0.0000000000000000000000000000000000001) len
> -    ok 137 - mp_decode(mp_encode(0.0000000000000000000000000000000000001)) value
> -    ok 138 - mp_decode(mp_encode(0.0000000000000000000000000000000000001)) scale
> -    ok 139 - str(mp_decode(mp_encode(0.0000000000000000000000000000000000001))) == 0.0000000000000000000000000000000000001
> -    ok 140 - mp_ext_type is MP_DECIMAL
> -    ok 141 - decimal_unpack() after mp_decode_extl()
> -    ok 142 - decimal_unpack() after mp_decode_extl() value
> -    ok 143 - decimal_unpack() after mp_decode_extl() len
> -    ok 144 - mp_sizeof_decimal(-0.0000000000000000000000000000000000001)
> -    ok 145 - mp_sizeof_decimal(-0.0000000000000000000000000000000000001) == len(mp_encode_decimal(-0.0000000000000000000000000000000000001))
> -    ok 146 - mp_next(mp_encode(-0.0000000000000000000000000000000000001))
> -    ok 147 - mp_decode(mp_encode(-0.0000000000000000000000000000000000001) len
> -    ok 148 - mp_decode(mp_encode(-0.0000000000000000000000000000000000001)) value
> -    ok 149 - mp_decode(mp_encode(-0.0000000000000000000000000000000000001)) scale
> -    ok 150 - str(mp_decode(mp_encode(-0.0000000000000000000000000000000000001))) == -0.0000000000000000000000000000000000001
> -    ok 151 - mp_ext_type is MP_DECIMAL
> -    ok 152 - decimal_unpack() after mp_decode_extl()
> -    ok 153 - decimal_unpack() after mp_decode_extl() value
> -    ok 154 - decimal_unpack() after mp_decode_extl() len
> -    ok 155 - mp_sizeof_decimal(0.00000000000000000000000000000000000001)
> -    ok 156 - mp_sizeof_decimal(0.00000000000000000000000000000000000001) == len(mp_encode_decimal(0.00000000000000000000000000000000000001))
> -    ok 157 - mp_next(mp_encode(0.00000000000000000000000000000000000001))
> -    ok 158 - mp_decode(mp_encode(0.00000000000000000000000000000000000001) len
> -    ok 159 - mp_decode(mp_encode(0.00000000000000000000000000000000000001)) value
> -    ok 160 - mp_decode(mp_encode(0.00000000000000000000000000000000000001)) scale
> -    ok 161 - str(mp_decode(mp_encode(0.00000000000000000000000000000000000001))) == 0.00000000000000000000000000000000000001
> -    ok 162 - mp_ext_type is MP_DECIMAL
> -    ok 163 - decimal_unpack() after mp_decode_extl()
> -    ok 164 - decimal_unpack() after mp_decode_extl() value
> -    ok 165 - decimal_unpack() after mp_decode_extl() len
> -    ok 166 - mp_sizeof_decimal(-0.00000000000000000000000000000000000001)
> -    ok 167 - mp_sizeof_decimal(-0.00000000000000000000000000000000000001) == len(mp_encode_decimal(-0.00000000000000000000000000000000000001))
> -    ok 168 - mp_next(mp_encode(-0.00000000000000000000000000000000000001))
> -    ok 169 - mp_decode(mp_encode(-0.00000000000000000000000000000000000001) len
> -    ok 170 - mp_decode(mp_encode(-0.00000000000000000000000000000000000001)) value
> -    ok 171 - mp_decode(mp_encode(-0.00000000000000000000000000000000000001)) scale
> -    ok 172 - str(mp_decode(mp_encode(-0.00000000000000000000000000000000000001))) == -0.00000000000000000000000000000000000001
> -    ok 173 - mp_ext_type is MP_DECIMAL
> -    ok 174 - decimal_unpack() after mp_decode_extl()
> -    ok 175 - decimal_unpack() after mp_decode_extl() value
> -    ok 176 - decimal_unpack() after mp_decode_extl() len
> -    ok 177 - mp_sizeof_decimal(99999999999999999999999999999999999999)
> -    ok 178 - mp_sizeof_decimal(99999999999999999999999999999999999999) == len(mp_encode_decimal(99999999999999999999999999999999999999))
> -    ok 179 - mp_next(mp_encode(99999999999999999999999999999999999999))
> -    ok 180 - mp_decode(mp_encode(99999999999999999999999999999999999999) len
> -    ok 181 - mp_decode(mp_encode(99999999999999999999999999999999999999)) value
> -    ok 182 - mp_decode(mp_encode(99999999999999999999999999999999999999)) scale
> -    ok 183 - str(mp_decode(mp_encode(99999999999999999999999999999999999999))) == 99999999999999999999999999999999999999
> -    ok 184 - mp_ext_type is MP_DECIMAL
> -    ok 185 - decimal_unpack() after mp_decode_extl()
> -    ok 186 - decimal_unpack() after mp_decode_extl() value
> -    ok 187 - decimal_unpack() after mp_decode_extl() len
> -    ok 188 - mp_sizeof_decimal(-99999999999999999999999999999999999999)
> -    ok 189 - mp_sizeof_decimal(-99999999999999999999999999999999999999) == len(mp_encode_decimal(-99999999999999999999999999999999999999))
> -    ok 190 - mp_next(mp_encode(-99999999999999999999999999999999999999))
> -    ok 191 - mp_decode(mp_encode(-99999999999999999999999999999999999999) len
> -    ok 192 - mp_decode(mp_encode(-99999999999999999999999999999999999999)) value
> -    ok 193 - mp_decode(mp_encode(-99999999999999999999999999999999999999)) scale
> -    ok 194 - str(mp_decode(mp_encode(-99999999999999999999999999999999999999))) == -99999999999999999999999999999999999999
> -    ok 195 - mp_ext_type is MP_DECIMAL
> -    ok 196 - decimal_unpack() after mp_decode_extl()
> -    ok 197 - decimal_unpack() after mp_decode_extl() value
> -    ok 198 - decimal_unpack() after mp_decode_extl() len
> +    ok 133 - mp_sizeof_decimal(-1234567891234567890.0987654321987654321)
> +    ok 134 - mp_sizeof_decimal(-1234567891234567890.0987654321987654321) == len(mp_encode_decimal(-1234567891234567890.0987654321987654321))
> +    ok 135 - mp_next(mp_encode(-1234567891234567890.0987654321987654321))
> +    ok 136 - mp_decode(mp_encode(-1234567891234567890.0987654321987654321) len
> +    ok 137 - mp_decode(mp_encode(-1234567891234567890.0987654321987654321)) value
> +    ok 138 - mp_decode(mp_encode(-1234567891234567890.0987654321987654321)) scale
> +    ok 139 - str(mp_decode(mp_encode(-1234567891234567890.0987654321987654321))) == -1234567891234567890.0987654321987654321
> +    ok 140 - stack str == static str for -1234567891234567890.0987654321987654321
> +    ok 141 - mp_ext_type is MP_DECIMAL
> +    ok 142 - decimal_unpack() after mp_decode_extl()
> +    ok 143 - decimal_unpack() after mp_decode_extl() value
> +    ok 144 - decimal_unpack() after mp_decode_extl() len
> +    ok 145 - mp_sizeof_decimal(0.0000000000000000000000000000000000001)
> +    ok 146 - mp_sizeof_decimal(0.0000000000000000000000000000000000001) == len(mp_encode_decimal(0.0000000000000000000000000000000000001))
> +    ok 147 - mp_next(mp_encode(0.0000000000000000000000000000000000001))
> +    ok 148 - mp_decode(mp_encode(0.0000000000000000000000000000000000001) len
> +    ok 149 - mp_decode(mp_encode(0.0000000000000000000000000000000000001)) value
> +    ok 150 - mp_decode(mp_encode(0.0000000000000000000000000000000000001)) scale
> +    ok 151 - str(mp_decode(mp_encode(0.0000000000000000000000000000000000001))) == 0.0000000000000000000000000000000000001
> +    ok 152 - stack str == static str for 0.0000000000000000000000000000000000001
> +    ok 153 - mp_ext_type is MP_DECIMAL
> +    ok 154 - decimal_unpack() after mp_decode_extl()
> +    ok 155 - decimal_unpack() after mp_decode_extl() value
> +    ok 156 - decimal_unpack() after mp_decode_extl() len
> +    ok 157 - mp_sizeof_decimal(-0.0000000000000000000000000000000000001)
> +    ok 158 - mp_sizeof_decimal(-0.0000000000000000000000000000000000001) == len(mp_encode_decimal(-0.0000000000000000000000000000000000001))
> +    ok 159 - mp_next(mp_encode(-0.0000000000000000000000000000000000001))
> +    ok 160 - mp_decode(mp_encode(-0.0000000000000000000000000000000000001) len
> +    ok 161 - mp_decode(mp_encode(-0.0000000000000000000000000000000000001)) value
> +    ok 162 - mp_decode(mp_encode(-0.0000000000000000000000000000000000001)) scale
> +    ok 163 - str(mp_decode(mp_encode(-0.0000000000000000000000000000000000001))) == -0.0000000000000000000000000000000000001
> +    ok 164 - stack str == static str for -0.0000000000000000000000000000000000001
> +    ok 165 - mp_ext_type is MP_DECIMAL
> +    ok 166 - decimal_unpack() after mp_decode_extl()
> +    ok 167 - decimal_unpack() after mp_decode_extl() value
> +    ok 168 - decimal_unpack() after mp_decode_extl() len
> +    ok 169 - mp_sizeof_decimal(0.00000000000000000000000000000000000001)
> +    ok 170 - mp_sizeof_decimal(0.00000000000000000000000000000000000001) == len(mp_encode_decimal(0.00000000000000000000000000000000000001))
> +    ok 171 - mp_next(mp_encode(0.00000000000000000000000000000000000001))
> +    ok 172 - mp_decode(mp_encode(0.00000000000000000000000000000000000001) len
> +    ok 173 - mp_decode(mp_encode(0.00000000000000000000000000000000000001)) value
> +    ok 174 - mp_decode(mp_encode(0.00000000000000000000000000000000000001)) scale
> +    ok 175 - str(mp_decode(mp_encode(0.00000000000000000000000000000000000001))) == 0.00000000000000000000000000000000000001
> +    ok 176 - stack str == static str for 0.00000000000000000000000000000000000001
> +    ok 177 - mp_ext_type is MP_DECIMAL
> +    ok 178 - decimal_unpack() after mp_decode_extl()
> +    ok 179 - decimal_unpack() after mp_decode_extl() value
> +    ok 180 - decimal_unpack() after mp_decode_extl() len
> +    ok 181 - mp_sizeof_decimal(-0.00000000000000000000000000000000000001)
> +    ok 182 - mp_sizeof_decimal(-0.00000000000000000000000000000000000001) == len(mp_encode_decimal(-0.00000000000000000000000000000000000001))
> +    ok 183 - mp_next(mp_encode(-0.00000000000000000000000000000000000001))
> +    ok 184 - mp_decode(mp_encode(-0.00000000000000000000000000000000000001) len
> +    ok 185 - mp_decode(mp_encode(-0.00000000000000000000000000000000000001)) value
> +    ok 186 - mp_decode(mp_encode(-0.00000000000000000000000000000000000001)) scale
> +    ok 187 - str(mp_decode(mp_encode(-0.00000000000000000000000000000000000001))) == -0.00000000000000000000000000000000000001
> +    ok 188 - stack str == static str for -0.00000000000000000000000000000000000001
> +    ok 189 - mp_ext_type is MP_DECIMAL
> +    ok 190 - decimal_unpack() after mp_decode_extl()
> +    ok 191 - decimal_unpack() after mp_decode_extl() value
> +    ok 192 - decimal_unpack() after mp_decode_extl() len
> +    ok 193 - mp_sizeof_decimal(99999999999999999999999999999999999999)
> +    ok 194 - mp_sizeof_decimal(99999999999999999999999999999999999999) == len(mp_encode_decimal(99999999999999999999999999999999999999))
> +    ok 195 - mp_next(mp_encode(99999999999999999999999999999999999999))
> +    ok 196 - mp_decode(mp_encode(99999999999999999999999999999999999999) len
> +    ok 197 - mp_decode(mp_encode(99999999999999999999999999999999999999)) value
> +    ok 198 - mp_decode(mp_encode(99999999999999999999999999999999999999)) scale
> +    ok 199 - str(mp_decode(mp_encode(99999999999999999999999999999999999999))) == 99999999999999999999999999999999999999
> +    ok 200 - stack str == static str for 99999999999999999999999999999999999999
> +    ok 201 - mp_ext_type is MP_DECIMAL
> +    ok 202 - decimal_unpack() after mp_decode_extl()
> +    ok 203 - decimal_unpack() after mp_decode_extl() value
> +    ok 204 - decimal_unpack() after mp_decode_extl() len
> +    ok 205 - mp_sizeof_decimal(-99999999999999999999999999999999999999)
> +    ok 206 - mp_sizeof_decimal(-99999999999999999999999999999999999999) == len(mp_encode_decimal(-99999999999999999999999999999999999999))
> +    ok 207 - mp_next(mp_encode(-99999999999999999999999999999999999999))
> +    ok 208 - mp_decode(mp_encode(-99999999999999999999999999999999999999) len
> +    ok 209 - mp_decode(mp_encode(-99999999999999999999999999999999999999)) value
> +    ok 210 - mp_decode(mp_encode(-99999999999999999999999999999999999999)) scale
> +    ok 211 - str(mp_decode(mp_encode(-99999999999999999999999999999999999999))) == -99999999999999999999999999999999999999
> +    ok 212 - stack str == static str for -99999999999999999999999999999999999999
> +    ok 213 - mp_ext_type is MP_DECIMAL
> +    ok 214 - decimal_unpack() after mp_decode_extl()
> +    ok 215 - decimal_unpack() after mp_decode_extl() value
> +    ok 216 - decimal_unpack() after mp_decode_extl() len
> ok 281 - subtests
>     1..5
> 	*** test_mp_print ***
> -- 
> 2.24.3 (Apple Git-128)
> 


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Tarantool-patches] [PATCH 4/5] decimal: introduce decimal_to_string
  2021-07-29 11:52   ` Sergey Ostanevich via Tarantool-patches
@ 2021-08-01 15:04     ` Vladislav Shpilevoy via Tarantool-patches
  2021-08-01 17:06       ` Sergey Ostanevich via Tarantool-patches
  0 siblings, 1 reply; 20+ messages in thread
From: Vladislav Shpilevoy via Tarantool-patches @ 2021-08-01 15:04 UTC (permalink / raw)
  To: Sergey Ostanevich; +Cc: tarantool-patches

Thanks for the review!

On 29.07.2021 13:52, Sergey Ostanevich wrote:
> Hi! Thanks for the patch, my 2 cents below. 
> 
> Otherwise - LGTM.
> 
> Regards,
> Sergos
> 
> 
>> On 28 Jul 2021, at 00:24, Vladislav Shpilevoy <v.shpilevoy@tarantool.org> wrote:
>>
>> It saves decimal as a string into an externally passed buffer.
> 
> Please mention decimal_to_string() introduction.

It is in the commit title already. I don't want to duplicate the title
in the message.

>> diff --git a/src/lib/core/decimal.c b/src/lib/core/decimal.c
>> index 4befbda37..6d2ccb96f 100644
>> --- a/src/lib/core/decimal.c
>> +++ b/src/lib/core/decimal.c
>> @@ -175,6 +175,14 @@ decimal_str(const decimal_t *dec)
>> 	return buf;
>> }
>>
>> +void
>> +decimal_to_string(const decimal_t *dec, char *str)
>> +{
>> +	char *tmp = decNumberToString(dec, str);
>> +	assert(str == tmp);
> 
> Why? The decToString() doesn't touch original string pointer, working on
> a local copy.

Yes, this is why there is the assertion. Also to be consistent with
decimal_str() which has the same assert.

>> +	(void)tmp;
>> +}

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Tarantool-patches] [PATCH 4/5] decimal: introduce decimal_to_string
  2021-08-01 15:04     ` Vladislav Shpilevoy via Tarantool-patches
@ 2021-08-01 17:06       ` Sergey Ostanevich via Tarantool-patches
  0 siblings, 0 replies; 20+ messages in thread
From: Sergey Ostanevich via Tarantool-patches @ 2021-08-01 17:06 UTC (permalink / raw)
  To: Vladislav Shpilevoy; +Cc: tarantool-patches

Hi! Thanks for updates!

> 
> It is in the commit title already. I don't want to duplicate the title
> in the message.
> 

Sure, just missed it. 

>>> diff --git a/src/lib/core/decimal.c b/src/lib/core/decimal.c
>>> index 4befbda37..6d2ccb96f 100644
>>> --- a/src/lib/core/decimal.c
>>> +++ b/src/lib/core/decimal.c
>>> @@ -175,6 +175,14 @@ decimal_str(const decimal_t *dec)
>>> 	return buf;
>>> }
>>> 
>>> +void
>>> +decimal_to_string(const decimal_t *dec, char *str)
>>> +{
>>> +	char *tmp = decNumberToString(dec, str);
>>> +	assert(str == tmp);
>> 
>> Why? The decToString() doesn't touch original string pointer, working on
>> a local copy.
> 
> Yes, this is why there is the assertion. Also to be consistent with
> decimal_str() which has the same assert.
> 

I really doubt we need to assert something that has no chance to be wrong.

Anyways, LGTM.

Sergos


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [Tarantool-patches] [PATCH 5/5] decimal: introduce and use lua_pushdecimalstr()
  2021-07-27 21:24 [Tarantool-patches] [PATCH 0/5] Static buf in Lua, part 3 Vladislav Shpilevoy via Tarantool-patches
                   ` (3 preceding siblings ...)
  2021-07-27 21:24 ` [Tarantool-patches] [PATCH 4/5] decimal: introduce decimal_to_string Vladislav Shpilevoy via Tarantool-patches
@ 2021-07-27 21:24 ` Vladislav Shpilevoy via Tarantool-patches
  2021-07-29 12:28   ` Sergey Ostanevich via Tarantool-patches
  2021-07-27 21:39 ` [Tarantool-patches] [PATCH 0/5] Static buf in Lua, part 3 Cyrill Gorcunov via Tarantool-patches
  2021-08-02 19:45 ` Vladislav Shpilevoy via Tarantool-patches
  6 siblings, 1 reply; 20+ messages in thread
From: Vladislav Shpilevoy via Tarantool-patches @ 2021-07-27 21:24 UTC (permalink / raw)
  To: tarantool-patches, gorcunov, sergos

decimal conversion to string in Lua used decimal_str() function.
The function is not safe to use in preemptive context like Lua,
where any attempt to push something onto the Lua stack might
trigger GC, which in turn might invoke any other code.

It is not safe because uses the static buffer, which is global and
cyclic. Newer allocations can override the old data without any
warning.

The same problem was fixed for tt_uuid_str() and uuids in
box.info in one of the previous commits.

The patch adds a new function lua_pushdecimalstr() which does not
use the static buffer. It is now used to push decimals safely on a
Lua stack.

Follow up #5632
Follow up #6050
Closes #6259
---
 .../unreleased/gh-6259-static-buf-in-lua.md   |  5 +++
 src/lua/decimal.c                             | 14 ++++++-
 src/lua/decimal.h                             |  3 ++
 .../gh-5632-6050-6259-gc-buf-reuse.test.lua   | 39 ++++++++++++++++++-
 4 files changed, 59 insertions(+), 2 deletions(-)
 create mode 100644 changelogs/unreleased/gh-6259-static-buf-in-lua.md

diff --git a/changelogs/unreleased/gh-6259-static-buf-in-lua.md b/changelogs/unreleased/gh-6259-static-buf-in-lua.md
new file mode 100644
index 000000000..1a2f7705a
--- /dev/null
+++ b/changelogs/unreleased/gh-6259-static-buf-in-lua.md
@@ -0,0 +1,5 @@
+## bugfix/core
+
+ * `box.info.uuid`, `box.info.cluster.uuid`, and `tostring(decimal)` with any
+   decimal number in Lua sometimes could return garbage if `__gc` handlers are
+   used in user's code (gh-6259).
diff --git a/src/lua/decimal.c b/src/lua/decimal.c
index e25b3ec18..a50e4142a 100644
--- a/src/lua/decimal.c
+++ b/src/lua/decimal.c
@@ -89,6 +89,18 @@ lua_pushdecimal(struct lua_State *L)
 	return res;
 }
 
+void
+lua_pushdecimalstr(struct lua_State *L, const decimal_t *dec)
+{
+	/*
+	 * Do not use a global buffer. It might be overwritten if GC starts
+	 * working.
+	 */
+	char str[DECIMAL_MAX_STR_LEN + 1];
+	decimal_to_string(dec, str);
+	lua_pushstring(L, str);
+}
+
 /**
  * Returns true if a value at a given index is a decimal
  * and false otherwise
@@ -376,7 +388,7 @@ ldecimal_tostring(struct lua_State *L)
 	if (lua_gettop(L) < 1)
 		return luaL_error(L, "usage: decimal.tostring(decimal)");
 	decimal_t *lhs = lua_checkdecimal(L, 1);
-	lua_pushstring(L, decimal_str(lhs));
+	lua_pushdecimalstr(L, lhs);
 	return 1;
 }
 
diff --git a/src/lua/decimal.h b/src/lua/decimal.h
index b13e59247..59a71c312 100644
--- a/src/lua/decimal.h
+++ b/src/lua/decimal.h
@@ -44,6 +44,9 @@ struct lua_State;
 decimal_t *
 lua_pushdecimal(struct lua_State *L);
 
+void
+lua_pushdecimalstr(struct lua_State *L, const decimal_t *dec);
+
 void
 tarantool_lua_decimal_init(struct lua_State *L);
 
diff --git a/test/app-tap/gh-5632-6050-6259-gc-buf-reuse.test.lua b/test/app-tap/gh-5632-6050-6259-gc-buf-reuse.test.lua
index f806ba6b7..45273ed9a 100755
--- a/test/app-tap/gh-5632-6050-6259-gc-buf-reuse.test.lua
+++ b/test/app-tap/gh-5632-6050-6259-gc-buf-reuse.test.lua
@@ -13,6 +13,7 @@ local uuid = require('uuid')
 local uri = require('uri')
 local json = require('json')
 local msgpackffi = require('msgpackffi')
+local decimal = require('decimal')
 
 local function test_uuid(test)
     test:plan(1)
@@ -223,14 +224,50 @@ local function test_info_uuid(test)
     test:ok(is_success, 'info uuid in gc')
 end
 
+local function test_decimal(test)
+    test:plan(1)
+
+    local gc_count = 100
+    local iter_count = 1000
+    local is_success = true
+
+    local d1 = decimal.new(1111111111111111111)
+    local d2 = decimal.new(2222222222222222222)
+
+    local function decimal_to_str()
+        local str1 = tostring(d1)
+        local str2 = tostring(d2)
+        local str3 = tostring(d1)
+        local str4 = tostring(d2)
+        if str1 ~= str3 or str2 ~= str4 then
+            is_success = false
+            assert(false)
+        end
+    end
+
+    local function create_gc()
+        for _ = 1, gc_count do
+            ffi.gc(ffi.new('char[1]'), function() decimal_to_str() end)
+        end
+    end
+
+    for _ = 1, iter_count do
+        create_gc()
+        decimal_to_str()
+    end
+
+    test:ok(is_success, 'decimal str in gc')
+end
+
 box.cfg{}
 
 local test = tap.test('gh-5632-6050-6259-gc-buf-reuse')
-test:plan(5)
+test:plan(6)
 test:test('uuid in __gc', test_uuid)
 test:test('uri in __gc', test_uri)
 test:test('msgpackffi in __gc', test_msgpackffi)
 test:test('json in __gc', test_json)
 test:test('info uuid in __gc', test_info_uuid)
+test:test('decimal str in __gc', test_decimal)
 
 os.exit(test:check() and 0 or 1)
-- 
2.24.3 (Apple Git-128)


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Tarantool-patches] [PATCH 5/5] decimal: introduce and use lua_pushdecimalstr()
  2021-07-27 21:24 ` [Tarantool-patches] [PATCH 5/5] decimal: introduce and use lua_pushdecimalstr() Vladislav Shpilevoy via Tarantool-patches
@ 2021-07-29 12:28   ` Sergey Ostanevich via Tarantool-patches
  2021-08-01 15:04     ` Vladislav Shpilevoy via Tarantool-patches
  0 siblings, 1 reply; 20+ messages in thread
From: Sergey Ostanevich via Tarantool-patches @ 2021-07-29 12:28 UTC (permalink / raw)
  To: Vladislav Shpilevoy; +Cc: tarantool-patches

[-- Attachment #1: Type: text/plain, Size: 5454 bytes --]

Hi! Thanks for the patch!

Just a minor on assert in test, LGTM if resolved similar to uuid patch.


regards,
Sergos

> On 28 Jul 2021, at 00:24, Vladislav Shpilevoy <v.shpilevoy@tarantool.org> wrote:
> 
> decimal conversion to string in Lua used decimal_str() function.
> The function is not safe to use in preemptive context like Lua,
> where any attempt to push something onto the Lua stack might
> trigger GC, which in turn might invoke any other code.
> 
> It is not safe because uses the static buffer, which is global and
> cyclic. Newer allocations can override the old data without any
> warning.
> 
> The same problem was fixed for tt_uuid_str() and uuids in
> box.info in one of the previous commits.
> 
> The patch adds a new function lua_pushdecimalstr() which does not
> use the static buffer. It is now used to push decimals safely on a
> Lua stack.
> 
> Follow up #5632
> Follow up #6050
> Closes #6259
> ---
> .../unreleased/gh-6259-static-buf-in-lua.md   |  5 +++
> src/lua/decimal.c                             | 14 ++++++-
> src/lua/decimal.h                             |  3 ++
> .../gh-5632-6050-6259-gc-buf-reuse.test.lua   | 39 ++++++++++++++++++-
> 4 files changed, 59 insertions(+), 2 deletions(-)
> create mode 100644 changelogs/unreleased/gh-6259-static-buf-in-lua.md
> 
> diff --git a/changelogs/unreleased/gh-6259-static-buf-in-lua.md b/changelogs/unreleased/gh-6259-static-buf-in-lua.md
> new file mode 100644
> index 000000000..1a2f7705a
> --- /dev/null
> +++ b/changelogs/unreleased/gh-6259-static-buf-in-lua.md
> @@ -0,0 +1,5 @@
> +## bugfix/core
> +
> + * `box.info.uuid`, `box.info.cluster.uuid`, and `tostring(decimal)` with any
> +   decimal number in Lua sometimes could return garbage if `__gc` handlers are
> +   used in user's code (gh-6259).
> diff --git a/src/lua/decimal.c b/src/lua/decimal.c
> index e25b3ec18..a50e4142a 100644
> --- a/src/lua/decimal.c
> +++ b/src/lua/decimal.c
> @@ -89,6 +89,18 @@ lua_pushdecimal(struct lua_State *L)
> 	return res;
> }
> 
> +void
> +lua_pushdecimalstr(struct lua_State *L, const decimal_t *dec)
> +{
> +	/*
> +	 * Do not use a global buffer. It might be overwritten if GC starts
> +	 * working.
> +	 */
> +	char str[DECIMAL_MAX_STR_LEN + 1];
> +	decimal_to_string(dec, str);
> +	lua_pushstring(L, str);
> +}
> +
> /**
>  * Returns true if a value at a given index is a decimal
>  * and false otherwise
> @@ -376,7 +388,7 @@ ldecimal_tostring(struct lua_State *L)
> 	if (lua_gettop(L) < 1)
> 		return luaL_error(L, "usage: decimal.tostring(decimal)");
> 	decimal_t *lhs = lua_checkdecimal(L, 1);
> -	lua_pushstring(L, decimal_str(lhs));
> +	lua_pushdecimalstr(L, lhs);
> 	return 1;
> }
> 
> diff --git a/src/lua/decimal.h b/src/lua/decimal.h
> index b13e59247..59a71c312 100644
> --- a/src/lua/decimal.h
> +++ b/src/lua/decimal.h
> @@ -44,6 +44,9 @@ struct lua_State;
> decimal_t *
> lua_pushdecimal(struct lua_State *L);
> 
> +void
> +lua_pushdecimalstr(struct lua_State *L, const decimal_t *dec);
> +
> void
> tarantool_lua_decimal_init(struct lua_State *L);
> 
> diff --git a/test/app-tap/gh-5632-6050-6259-gc-buf-reuse.test.lua b/test/app-tap/gh-5632-6050-6259-gc-buf-reuse.test.lua
> index f806ba6b7..45273ed9a 100755
> --- a/test/app-tap/gh-5632-6050-6259-gc-buf-reuse.test.lua
> +++ b/test/app-tap/gh-5632-6050-6259-gc-buf-reuse.test.lua
> @@ -13,6 +13,7 @@ local uuid = require('uuid')
> local uri = require('uri')
> local json = require('json')
> local msgpackffi = require('msgpackffi')
> +local decimal = require('decimal')
> 
> local function test_uuid(test)
>     test:plan(1)
> @@ -223,14 +224,50 @@ local function test_info_uuid(test)
>     test:ok(is_success, 'info uuid in gc')
> end
> 
> +local function test_decimal(test)
> +    test:plan(1)
> +
> +    local gc_count = 100
> +    local iter_count = 1000
> +    local is_success = true
> +
> +    local d1 = decimal.new(1111111111111111111)
> +    local d2 = decimal.new(2222222222222222222)
> +
> +    local function decimal_to_str()
> +        local str1 = tostring(d1)
> +        local str2 = tostring(d2)
> +        local str3 = tostring(d1)
> +        local str4 = tostring(d2)
> +        if str1 ~= str3 or str2 ~= str4 then
> +            is_success = false
> +            assert(false)

Same as previos patch with uuid. I refer to the
https://www.tarantool.io/en/doc/latest/reference/reference_lua/tap/#tap-example <https://www.tarantool.io/en/doc/latest/reference/reference_lua/tap/#tap-example>
perhaps I miss something?

> +        end
> +    end
> +
> +    local function create_gc()
> +        for _ = 1, gc_count do
> +            ffi.gc(ffi.new('char[1]'), function() decimal_to_str() end)
> +        end
> +    end
> +
> +    for _ = 1, iter_count do
> +        create_gc()
> +        decimal_to_str()
> +    end
> +
> +    test:ok(is_success, 'decimal str in gc')
> +end
> +
> box.cfg{}
> 
> local test = tap.test('gh-5632-6050-6259-gc-buf-reuse')
> -test:plan(5)
> +test:plan(6)
> test:test('uuid in __gc', test_uuid)
> test:test('uri in __gc', test_uri)
> test:test('msgpackffi in __gc', test_msgpackffi)
> test:test('json in __gc', test_json)
> test:test('info uuid in __gc', test_info_uuid)
> +test:test('decimal str in __gc', test_decimal)
> 
> os.exit(test:check() and 0 or 1)
> -- 
> 2.24.3 (Apple Git-128)
> 


[-- Attachment #2: Type: text/html, Size: 9649 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Tarantool-patches] [PATCH 5/5] decimal: introduce and use lua_pushdecimalstr()
  2021-07-29 12:28   ` Sergey Ostanevich via Tarantool-patches
@ 2021-08-01 15:04     ` Vladislav Shpilevoy via Tarantool-patches
  0 siblings, 0 replies; 20+ messages in thread
From: Vladislav Shpilevoy via Tarantool-patches @ 2021-08-01 15:04 UTC (permalink / raw)
  To: Sergey Ostanevich; +Cc: tarantool-patches

Thanks for the review!

>> diff --git a/test/app-tap/gh-5632-6050-6259-gc-buf-reuse.test.lua b/test/app-tap/gh-5632-6050-6259-gc-buf-reuse.test.lua
>> index f806ba6b7..45273ed9a 100755
>> --- a/test/app-tap/gh-5632-6050-6259-gc-buf-reuse.test.lua
>> +++ b/test/app-tap/gh-5632-6050-6259-gc-buf-reuse.test.lua
>> @@ -223,14 +224,50 @@ local function test_info_uuid(test)
>>     test:ok(is_success, 'info uuid in gc')
>> end
>>
>> +local function test_decimal(test)
>> +    test:plan(1)
>> +
>> +    local gc_count = 100
>> +    local iter_count = 1000
>> +    local is_success = true
>> +
>> +    local d1 = decimal.new(1111111111111111111)
>> +    local d2 = decimal.new(2222222222222222222)
>> +
>> +    local function decimal_to_str()
>> +        local str1 = tostring(d1)
>> +        local str2 = tostring(d2)
>> +        local str3 = tostring(d1)
>> +        local str4 = tostring(d2)
>> +        if str1 ~= str3 or str2 ~= str4 then
>> +            is_success = false
>> +            assert(false)
> 
> Same as previos patch with uuid. I refer to the
> https://www.tarantool.io/en/doc/latest/reference/reference_lua/tap/#tap-example <https://www.tarantool.io/en/doc/latest/reference/reference_lua/tap/#tap-example>
> perhaps I miss something?

The assert is dropped on the branch.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Tarantool-patches] [PATCH 0/5] Static buf in Lua, part 3
  2021-07-27 21:24 [Tarantool-patches] [PATCH 0/5] Static buf in Lua, part 3 Vladislav Shpilevoy via Tarantool-patches
                   ` (4 preceding siblings ...)
  2021-07-27 21:24 ` [Tarantool-patches] [PATCH 5/5] decimal: introduce and use lua_pushdecimalstr() Vladislav Shpilevoy via Tarantool-patches
@ 2021-07-27 21:39 ` Cyrill Gorcunov via Tarantool-patches
  2021-08-02 19:45 ` Vladislav Shpilevoy via Tarantool-patches
  6 siblings, 0 replies; 20+ messages in thread
From: Cyrill Gorcunov via Tarantool-patches @ 2021-07-27 21:39 UTC (permalink / raw)
  To: Vladislav Shpilevoy; +Cc: tarantool-patches

On Tue, Jul 27, 2021 at 11:24:10PM +0200, Vladislav Shpilevoy wrote:
> Here we go again.
> 
> I see that Lua UUID helpers have L suffix and decimal helpers do not. Have no
> idea why, but decided not to change it here since it is not related to the patch
> goal.
> 
> I validated other usages of tt_uuid_str(), decimal_str(), tt_sprintf(),
> tt_statuc_buf(), tuple_str(), mp_str() and some others - they all seem to be
> fine now. But you can never say for sure.
> 
> Branch: http://github.com/tarantool/tarantool/tree/gerold103/gh-6259-static_buf-part3
> Issue: https://github.com/tarantool/tarantool/issues/6259

As to me the helpers

inline void
tt_uuid_to_string(const struct tt_uuid *uu, char *out)
{
	snprintf(out, UUID_STR_LEN + 1,
		"%08x-%04x-%04x-%02x%02x-%02x%02x%02x%02x%02x%02x",
		uu->time_low, uu->time_mid, uu->time_hi_and_version,
		uu->clock_seq_hi_and_reserved, uu->clock_seq_low, uu->node[0],
		uu->node[1], uu->node[2], uu->node[3], uu->node[4], uu->node[5]);
}

should had been banned from the begining, length of output buffer
must be explicitly passed instead, ie

	buf[UUID_STR_LEN+1];
	tt_uuid_to_string(uu, buf, sizeof(buf));

but this is obviously unrelated to the series itself just wonder who
ever came up with such api at all, since it is just a bug lurking
around waiting to zap the caller's stack.

The series is good, thanks Vlad! Ack.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Tarantool-patches] [PATCH 0/5] Static buf in Lua, part 3
  2021-07-27 21:24 [Tarantool-patches] [PATCH 0/5] Static buf in Lua, part 3 Vladislav Shpilevoy via Tarantool-patches
                   ` (5 preceding siblings ...)
  2021-07-27 21:39 ` [Tarantool-patches] [PATCH 0/5] Static buf in Lua, part 3 Cyrill Gorcunov via Tarantool-patches
@ 2021-08-02 19:45 ` Vladislav Shpilevoy via Tarantool-patches
  6 siblings, 0 replies; 20+ messages in thread
From: Vladislav Shpilevoy via Tarantool-patches @ 2021-08-02 19:45 UTC (permalink / raw)
  To: tarantool-patches, gorcunov, sergos

Pushed to master, 2.8, 2.7, 1.10.

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2021-08-02 19:45 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-27 21:24 [Tarantool-patches] [PATCH 0/5] Static buf in Lua, part 3 Vladislav Shpilevoy via Tarantool-patches
2021-07-27 21:24 ` [Tarantool-patches] [PATCH 1/5] uuid: introduce and use luaL_pushuuidstr() Vladislav Shpilevoy via Tarantool-patches
2021-07-29 11:30   ` Sergey Ostanevich via Tarantool-patches
2021-07-27 21:24 ` [Tarantool-patches] [PATCH 2/5] info: use luaL_pushuuidstr() for box.info uuids Vladislav Shpilevoy via Tarantool-patches
2021-07-29 11:38   ` Sergey Ostanevich via Tarantool-patches
2021-08-01 15:03     ` Vladislav Shpilevoy via Tarantool-patches
2021-08-01 17:01       ` Sergey Ostanevich via Tarantool-patches
2021-07-27 21:24 ` [Tarantool-patches] [PATCH 3/5] decimal: rename decimal_to_string to decimal_str Vladislav Shpilevoy via Tarantool-patches
2021-07-29 11:41   ` Sergey Ostanevich via Tarantool-patches
2021-08-01 15:03     ` Vladislav Shpilevoy via Tarantool-patches
2021-08-01 17:01       ` Sergey Ostanevich via Tarantool-patches
2021-07-27 21:24 ` [Tarantool-patches] [PATCH 4/5] decimal: introduce decimal_to_string Vladislav Shpilevoy via Tarantool-patches
2021-07-29 11:52   ` Sergey Ostanevich via Tarantool-patches
2021-08-01 15:04     ` Vladislav Shpilevoy via Tarantool-patches
2021-08-01 17:06       ` Sergey Ostanevich via Tarantool-patches
2021-07-27 21:24 ` [Tarantool-patches] [PATCH 5/5] decimal: introduce and use lua_pushdecimalstr() Vladislav Shpilevoy via Tarantool-patches
2021-07-29 12:28   ` Sergey Ostanevich via Tarantool-patches
2021-08-01 15:04     ` Vladislav Shpilevoy via Tarantool-patches
2021-07-27 21:39 ` [Tarantool-patches] [PATCH 0/5] Static buf in Lua, part 3 Cyrill Gorcunov via Tarantool-patches
2021-08-02 19:45 ` Vladislav Shpilevoy via Tarantool-patches

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox