Tarantool development patches archive
 help / color / mirror / Atom feed
* [Tarantool-patches] [PATCH v2 1/1] fiber: extend max fiber name length to 255
@ 2020-03-15 15:11 Vladislav Shpilevoy
  2020-03-16  7:19 ` Cyrill Gorcunov
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Vladislav Shpilevoy @ 2020-03-15 15:11 UTC (permalink / raw)
  To: tarantool-patches, gorcunov

Users keep complaining about too short fiber name. New limit is
255, should be enough for any sane name.

Closes #4394

@TarantoolBot document
Title: fiber.name length limit.

It was 32, now it is 255. Besides, it seems like `fiber.name`
`{truncate = true}` option is not documented.

By default, if a new name is too long, `fiber.name(new_name)`
fails with an exception. To make it always succeed there is an
option 'truncate': `fiber.name(new_name, {truncate = true})`. It
truncates the name to the max length if it is too long.
---
Branch: http://github.com/tarantool/tarantool/tree/gerold103/gh-4394-fiber-name-inline
Issue: https://github.com/tarantool/tarantool/issues/4394

Changes in v2:
- Short fiber names are kept inside struct fiber.

 src/lib/core/fiber.c    | 28 ++++++++++++++++++++++++++--
 src/lib/core/fiber.h    | 14 +++++++++++---
 test/app/fiber.result   |  8 ++++----
 test/app/fiber.test.lua |  2 +-
 test/unit/fiber.cc      |  6 +++---
 test/unit/fiber.result  |  2 +-
 6 files changed, 46 insertions(+), 14 deletions(-)

diff --git a/src/lib/core/fiber.c b/src/lib/core/fiber.c
index ada7972cb..359ef1187 100644
--- a/src/lib/core/fiber.c
+++ b/src/lib/core/fiber.c
@@ -912,8 +912,26 @@ fiber_loop(MAYBE_UNUSED void *data)
 void
 fiber_set_name(struct fiber *fiber, const char *name)
 {
-	assert(name != NULL);
-	snprintf(fiber->name, sizeof(fiber->name), "%s", name);
+	size_t len = strlen(name);
+	if (len <= FIBER_NAME_INLINE) {
+		if (fiber->name != fiber->inline_name) {
+			free(fiber->name);
+			fiber->name = fiber->inline_name;
+		}
+	} else {
+		if (len > FIBER_NAME_MAX)
+			len = FIBER_NAME_MAX;
+		char *new_name;
+		if (fiber->name != fiber->inline_name)
+			new_name = realloc(fiber->name, len + 1);
+		else
+			new_name = malloc(len + 1);
+		if (new_name == NULL)
+			panic("fiber_set_name() failed with OOM");
+		fiber->name = new_name;
+	}
+	memcpy(fiber->name, name, len);
+	fiber->name[len] = 0;
 }
 
 static inline void *
@@ -1242,6 +1260,9 @@ fiber_destroy(struct cord *cord, struct fiber *f)
 	region_destroy(&f->gc);
 	fiber_stack_destroy(f, &cord->slabc);
 	diag_destroy(&f->diag);
+	if (f->name != f->inline_name)
+		free(f->name);
+	f->name = NULL;
 }
 
 void
@@ -1357,6 +1378,7 @@ cord_create(struct cord *cord, const char *name)
 	fiber_reset(&cord->sched);
 	diag_create(&cord->sched.diag);
 	region_create(&cord->sched.gc, &cord->slabc);
+	cord->sched.name = NULL;
 	fiber_set_name(&cord->sched, "sched");
 	cord->fiber = &cord->sched;
 
@@ -1405,6 +1427,8 @@ cord_destroy(struct cord *cord)
 	}
 	region_destroy(&cord->sched.gc);
 	diag_destroy(&cord->sched.diag);
+	if (cord->sched.name != cord->sched.inline_name)
+		free(cord->sched.name);
 	slab_cache_destroy(&cord->slabc);
 }
 
diff --git a/src/lib/core/fiber.h b/src/lib/core/fiber.h
index 8a3e5aef5..fedc37c87 100644
--- a/src/lib/core/fiber.h
+++ b/src/lib/core/fiber.h
@@ -111,7 +111,10 @@ struct cpu_stat {
 
 #endif /* ENABLE_FIBER_TOP */
 
-enum { FIBER_NAME_MAX = 32 };
+enum {
+	FIBER_NAME_INLINE = 32,
+	FIBER_NAME_MAX = 255
+};
 
 /**
  * Fiber ids [0; 100] are reserved.
@@ -510,7 +513,12 @@ struct fiber {
 	struct ipc_wait_pad *wait_pad;
 	/** Exception which caused this fiber's death. */
 	struct diag diag;
-	char name[FIBER_NAME_MAX + 1];
+	/**
+	 * Name points at inline_name in case it is short. Long
+	 * name is allocated on the heap.
+	 */
+	char *name;
+	char inline_name[FIBER_NAME_INLINE + 1];
 };
 
 /** Invoke on_stop triggers and delete them. */
@@ -578,7 +586,7 @@ struct cord {
 	struct slab_cache slabc;
 	/** The "main" fiber of this cord, the scheduler. */
 	struct fiber sched;
-	char name[FIBER_NAME_MAX];
+	char name[FIBER_NAME_INLINE + 1];
 };
 
 extern __thread struct cord *cord_ptr;
diff --git a/test/app/fiber.result b/test/app/fiber.result
index 6d9604ad8..7331f61b3 100644
--- a/test/app/fiber.result
+++ b/test/app/fiber.result
@@ -1186,7 +1186,7 @@ fiber = nil
 ---
 ...
 --
--- gh-2622, gh-4011: fiber.name() truncates new name.
+-- gh-2622, gh-4011, gh-4394: fiber.name() truncates new name.
 --
 fiber = require('fiber')
 ---
@@ -1214,14 +1214,14 @@ fiber.name(long_name, {truncate = true})
 ...
 fiber.name()
 ---
-- aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
+- aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
 ...
 f = fiber.self()
 ---
 ...
 fiber.name(f)
 ---
-- aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
+- aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
 ...
 fiber.name(f, 'new_name')
 ---
@@ -1239,7 +1239,7 @@ fiber.name(f, long_name, {truncate = true})
 ...
 fiber.name(f)
 ---
-- aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
+- aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
 ...
 --
 -- gh-3493 fiber.create() does not roll back memtx transaction
diff --git a/test/app/fiber.test.lua b/test/app/fiber.test.lua
index 6df210d9c..b8e9abc6e 100644
--- a/test/app/fiber.test.lua
+++ b/test/app/fiber.test.lua
@@ -504,7 +504,7 @@ test_run:cmd("setopt delimiter ''");
 fiber = nil
 
 --
--- gh-2622, gh-4011: fiber.name() truncates new name.
+-- gh-2622, gh-4011, gh-4394: fiber.name() truncates new name.
 --
 fiber = require('fiber')
 long_name = string.rep('a', 300)
diff --git a/test/unit/fiber.cc b/test/unit/fiber.cc
index 91f7d43f9..b0dfc9b14 100644
--- a/test/unit/fiber.cc
+++ b/test/unit/fiber.cc
@@ -171,9 +171,9 @@ fiber_name_test()
 
 	note("set new fiber name: %s.\n", fiber_name(fiber()));
 
-	const char *long_name = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
-		"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"\
-		"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa";
+	char long_name[FIBER_NAME_MAX + 30];
+	memset(long_name, 'a', sizeof(long_name));
+	long_name[sizeof(long_name) - 1] = 0;
 
 	fiber_set_name(fiber(), long_name);
 
diff --git a/test/unit/fiber.result b/test/unit/fiber.result
index 7c9f85dcd..a61e0a2b8 100644
--- a/test/unit/fiber.result
+++ b/test/unit/fiber.result
@@ -5,7 +5,7 @@ SystemError Failed to allocate 42 bytes in allocator for exception: Cannot alloc
 
 # set new fiber name: Horace.
 
-# fiber name is truncated: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.
+# fiber name is truncated: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.
 
 	*** fiber_name_test: done ***
 	*** fiber_join_test ***
-- 
2.21.1 (Apple Git-122.3)

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

* Re: [Tarantool-patches] [PATCH v2 1/1] fiber: extend max fiber name length to 255
  2020-03-15 15:11 [Tarantool-patches] [PATCH v2 1/1] fiber: extend max fiber name length to 255 Vladislav Shpilevoy
@ 2020-03-16  7:19 ` Cyrill Gorcunov
  2020-03-16 22:28   ` Vladislav Shpilevoy
  2020-03-16 12:48 ` Nikita Pettik
  2020-03-18 21:35 ` Vladislav Shpilevoy
  2 siblings, 1 reply; 10+ messages in thread
From: Cyrill Gorcunov @ 2020-03-16  7:19 UTC (permalink / raw)
  To: Vladislav Shpilevoy; +Cc: tarantool-patches

On Sun, Mar 15, 2020 at 04:11:14PM +0100, Vladislav Shpilevoy wrote:
...
> diff --git a/src/lib/core/fiber.c b/src/lib/core/fiber.c
> index ada7972cb..359ef1187 100644
> --- a/src/lib/core/fiber.c
> +++ b/src/lib/core/fiber.c
> @@ -912,8 +912,26 @@ fiber_loop(MAYBE_UNUSED void *data)
>  void
>  fiber_set_name(struct fiber *fiber, const char *name)
>  {
> -	assert(name != NULL);
> -	snprintf(fiber->name, sizeof(fiber->name), "%s", name);
> +	size_t len = strlen(name);
> +	if (len <= FIBER_NAME_INLINE) {
> +		if (fiber->name != fiber->inline_name) {
> +			free(fiber->name);
> +			fiber->name = fiber->inline_name;
> +		}
> +	} else {
> +		if (len > FIBER_NAME_MAX)
> +			len = FIBER_NAME_MAX;
> +		char *new_name;
> +		if (fiber->name != fiber->inline_name)
> +			new_name = realloc(fiber->name, len + 1);
> +		else
> +			new_name = malloc(len + 1);
> +		if (new_name == NULL)
> +			panic("fiber_set_name() failed with OOM");
> +		fiber->name = new_name;
> +	}
> +	memcpy(fiber->name, name, len);
> +	fiber->name[len] = 0;
>  }

Thank, Vlad! I like the patch. There is only one concern I have: for
some reason we has been defining faiber name as

char name[FIBER_NAME_MAX + 1];

where FIBER_NAME_MAX = 32 and finally this expands to "char name[33];"
I'm too lazy to find who exactly introduced this but it is bloody
wrong: compiler alings members to eliminate data access penalty, thus
_actually_ it will be defined as 8 multilier, ie 40 bytes.

Thus, if you don't mind I propose make FIBER_NAME_INLINE = 40
*including* string terminating zero.

Actually we can make it on top then. Up to you. Anyway

Reviewed-by: Cyrill Gorcunov <gorcunov@gmail.com>

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

* Re: [Tarantool-patches] [PATCH v2 1/1] fiber: extend max fiber name length to 255
  2020-03-15 15:11 [Tarantool-patches] [PATCH v2 1/1] fiber: extend max fiber name length to 255 Vladislav Shpilevoy
  2020-03-16  7:19 ` Cyrill Gorcunov
@ 2020-03-16 12:48 ` Nikita Pettik
  2020-03-16 22:29   ` Vladislav Shpilevoy
  2020-03-18 21:35 ` Vladislav Shpilevoy
  2 siblings, 1 reply; 10+ messages in thread
From: Nikita Pettik @ 2020-03-16 12:48 UTC (permalink / raw)
  To: Vladislav Shpilevoy; +Cc: tarantool-patches

On 15 Mar 16:11, Vladislav Shpilevoy wrote:
> Users keep complaining about too short fiber name. New limit is
> 255, should be enough for any sane name.
> 
> Closes #4394
> 
> @TarantoolBot document
> Title: fiber.name length limit.
> 
> It was 32, now it is 255. Besides, it seems like `fiber.name`
> `{truncate = true}` option is not documented.
> 
> By default, if a new name is too long, `fiber.name(new_name)`
> fails with an exception. To make it always succeed there is an
> option 'truncate': `fiber.name(new_name, {truncate = true})`. It
> truncates the name to the max length if it is too long.
> ---
> Branch: http://github.com/tarantool/tarantool/tree/gerold103/gh-4394-fiber-name-inline
> Issue: https://github.com/tarantool/tarantool/issues/4394
> 
> Changes in v2:
> - Short fiber names are kept inside struct fiber.
>

LGTM. Seems you forgot to add changelog for github release.
 

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

* Re: [Tarantool-patches] [PATCH v2 1/1] fiber: extend max fiber name length to 255
  2020-03-16  7:19 ` Cyrill Gorcunov
@ 2020-03-16 22:28   ` Vladislav Shpilevoy
  2020-03-16 22:31     ` Cyrill Gorcunov
  0 siblings, 1 reply; 10+ messages in thread
From: Vladislav Shpilevoy @ 2020-03-16 22:28 UTC (permalink / raw)
  To: Cyrill Gorcunov; +Cc: tarantool-patches

Hi! Thanks for the review!

> Thank, Vlad! I like the patch. There is only one concern I have: for
> some reason we has been defining faiber name as
> 
> char name[FIBER_NAME_MAX + 1];
> 
> where FIBER_NAME_MAX = 32 and finally this expands to "char name[33];"
> I'm too lazy to find who exactly introduced this but it is bloody
> wrong: compiler alings members to eliminate data access penalty, thus
> _actually_ it will be defined as 8 multilier, ie 40 bytes.
> 
> Thus, if you don't mind I propose make FIBER_NAME_INLINE = 40
> *including* string terminating zero.
> 
> Actually we can make it on top then. Up to you. Anyway
> 
> Reviewed-by: Cyrill Gorcunov <gorcunov@gmail.com>

Yes, good point.

Here is diff. The whole patch is in the bottom.

====================
diff --git a/src/lib/core/fiber.c b/src/lib/core/fiber.c
index 359ef1187..5389ce467 100644
--- a/src/lib/core/fiber.c
+++ b/src/lib/core/fiber.c
@@ -912,26 +912,27 @@ fiber_loop(MAYBE_UNUSED void *data)
 void
 fiber_set_name(struct fiber *fiber, const char *name)
 {
-	size_t len = strlen(name);
-	if (len <= FIBER_NAME_INLINE) {
+	size_t size = strlen(name) + 1;
+	if (size <= FIBER_NAME_INLINE) {
 		if (fiber->name != fiber->inline_name) {
 			free(fiber->name);
 			fiber->name = fiber->inline_name;
 		}
 	} else {
-		if (len > FIBER_NAME_MAX)
-			len = FIBER_NAME_MAX;
+		if (size > FIBER_NAME_MAX)
+			size = FIBER_NAME_MAX;
 		char *new_name;
 		if (fiber->name != fiber->inline_name)
-			new_name = realloc(fiber->name, len + 1);
+			new_name = realloc(fiber->name, size);
 		else
-			new_name = malloc(len + 1);
+			new_name = malloc(size);
 		if (new_name == NULL)
 			panic("fiber_set_name() failed with OOM");
 		fiber->name = new_name;
 	}
-	memcpy(fiber->name, name, len);
-	fiber->name[len] = 0;
+	--size;
+	memcpy(fiber->name, name, size);
+	fiber->name[size] = 0;
 }
 
 static inline void *
diff --git a/src/lib/core/fiber.h b/src/lib/core/fiber.h
index fedc37c87..cd9346a55 100644
--- a/src/lib/core/fiber.h
+++ b/src/lib/core/fiber.h
@@ -112,8 +112,9 @@ struct cpu_stat {
 #endif /* ENABLE_FIBER_TOP */
 
 enum {
-	FIBER_NAME_INLINE = 32,
-	FIBER_NAME_MAX = 255
+	/** Both limits include terminating 0. */
+	FIBER_NAME_INLINE = 40,
+	FIBER_NAME_MAX = 256
 };
 
 /**
@@ -518,7 +519,7 @@ struct fiber {
 	 * name is allocated on the heap.
 	 */
 	char *name;
-	char inline_name[FIBER_NAME_INLINE + 1];
+	char inline_name[FIBER_NAME_INLINE];
 };
 
 /** Invoke on_stop triggers and delete them. */
@@ -586,7 +587,7 @@ struct cord {
 	struct slab_cache slabc;
 	/** The "main" fiber of this cord, the scheduler. */
 	struct fiber sched;
-	char name[FIBER_NAME_INLINE + 1];
+	char name[FIBER_NAME_INLINE];
 };
 
 extern __thread struct cord *cord_ptr;
diff --git a/src/lua/fiber.c b/src/lua/fiber.c
index 575a020d0..330eb7e54 100644
--- a/src/lua/fiber.c
+++ b/src/lua/fiber.c
@@ -582,16 +582,16 @@ lbox_fiber_name(struct lua_State *L)
 	if (top == name_index || top == opts_index) {
 		/* Set name. */
 		const char *name = luaL_checkstring(L, name_index);
-		int name_len = strlen(name);
+		int name_size = strlen(name) + 1;
 		if (top == opts_index && lua_istable(L, opts_index)) {
 			lua_getfield(L, opts_index, "truncate");
 			/* Truncate the name if needed. */
 			if (lua_isboolean(L, -1) && lua_toboolean(L, -1) &&
-			    name_len > FIBER_NAME_MAX)
-				name_len = FIBER_NAME_MAX;
+			    name_size > FIBER_NAME_MAX)
+				name_size = FIBER_NAME_MAX;
 			lua_pop(L, 1);
 		}
-		if (name_len > FIBER_NAME_MAX)
+		if (name_size > FIBER_NAME_MAX)
 			luaL_error(L, "Fiber name is too long");
 		fiber_set_name(f, name);
 		return 0;
====================

The whole patch:

====================

    fiber: extend max fiber name length to 255
    
    Users keep complaining about too short fiber name. New limit is
    255, should be enough for any sane name.
    
    Closes #4394
    
    @TarantoolBot document
    Title: fiber.name length limit.
    
    It was 32, now it is 255. Besides, it seems like `fiber.name`
    `{truncate = true}` option is not documented.
    
    By default, if a new name is too long, `fiber.name(new_name)`
    fails with an exception. To make it always succeed there is an
    option 'truncate': `fiber.name(new_name, {truncate = true})`. It
    truncates the name to the max length if it is too long.

diff --git a/src/lib/core/fiber.c b/src/lib/core/fiber.c
index ada7972cb..5389ce467 100644
--- a/src/lib/core/fiber.c
+++ b/src/lib/core/fiber.c
@@ -912,8 +912,27 @@ fiber_loop(MAYBE_UNUSED void *data)
 void
 fiber_set_name(struct fiber *fiber, const char *name)
 {
-	assert(name != NULL);
-	snprintf(fiber->name, sizeof(fiber->name), "%s", name);
+	size_t size = strlen(name) + 1;
+	if (size <= FIBER_NAME_INLINE) {
+		if (fiber->name != fiber->inline_name) {
+			free(fiber->name);
+			fiber->name = fiber->inline_name;
+		}
+	} else {
+		if (size > FIBER_NAME_MAX)
+			size = FIBER_NAME_MAX;
+		char *new_name;
+		if (fiber->name != fiber->inline_name)
+			new_name = realloc(fiber->name, size);
+		else
+			new_name = malloc(size);
+		if (new_name == NULL)
+			panic("fiber_set_name() failed with OOM");
+		fiber->name = new_name;
+	}
+	--size;
+	memcpy(fiber->name, name, size);
+	fiber->name[size] = 0;
 }
 
 static inline void *
@@ -1242,6 +1261,9 @@ fiber_destroy(struct cord *cord, struct fiber *f)
 	region_destroy(&f->gc);
 	fiber_stack_destroy(f, &cord->slabc);
 	diag_destroy(&f->diag);
+	if (f->name != f->inline_name)
+		free(f->name);
+	f->name = NULL;
 }
 
 void
@@ -1357,6 +1379,7 @@ cord_create(struct cord *cord, const char *name)
 	fiber_reset(&cord->sched);
 	diag_create(&cord->sched.diag);
 	region_create(&cord->sched.gc, &cord->slabc);
+	cord->sched.name = NULL;
 	fiber_set_name(&cord->sched, "sched");
 	cord->fiber = &cord->sched;
 
@@ -1405,6 +1428,8 @@ cord_destroy(struct cord *cord)
 	}
 	region_destroy(&cord->sched.gc);
 	diag_destroy(&cord->sched.diag);
+	if (cord->sched.name != cord->sched.inline_name)
+		free(cord->sched.name);
 	slab_cache_destroy(&cord->slabc);
 }
 
diff --git a/src/lib/core/fiber.h b/src/lib/core/fiber.h
index 8a3e5aef5..cd9346a55 100644
--- a/src/lib/core/fiber.h
+++ b/src/lib/core/fiber.h
@@ -111,7 +111,11 @@ struct cpu_stat {
 
 #endif /* ENABLE_FIBER_TOP */
 
-enum { FIBER_NAME_MAX = 32 };
+enum {
+	/** Both limits include terminating 0. */
+	FIBER_NAME_INLINE = 40,
+	FIBER_NAME_MAX = 256
+};
 
 /**
  * Fiber ids [0; 100] are reserved.
@@ -510,7 +514,12 @@ struct fiber {
 	struct ipc_wait_pad *wait_pad;
 	/** Exception which caused this fiber's death. */
 	struct diag diag;
-	char name[FIBER_NAME_MAX + 1];
+	/**
+	 * Name points at inline_name in case it is short. Long
+	 * name is allocated on the heap.
+	 */
+	char *name;
+	char inline_name[FIBER_NAME_INLINE];
 };
 
 /** Invoke on_stop triggers and delete them. */
@@ -578,7 +587,7 @@ struct cord {
 	struct slab_cache slabc;
 	/** The "main" fiber of this cord, the scheduler. */
 	struct fiber sched;
-	char name[FIBER_NAME_MAX];
+	char name[FIBER_NAME_INLINE];
 };
 
 extern __thread struct cord *cord_ptr;
diff --git a/src/lua/fiber.c b/src/lua/fiber.c
index 575a020d0..330eb7e54 100644
--- a/src/lua/fiber.c
+++ b/src/lua/fiber.c
@@ -582,16 +582,16 @@ lbox_fiber_name(struct lua_State *L)
 	if (top == name_index || top == opts_index) {
 		/* Set name. */
 		const char *name = luaL_checkstring(L, name_index);
-		int name_len = strlen(name);
+		int name_size = strlen(name) + 1;
 		if (top == opts_index && lua_istable(L, opts_index)) {
 			lua_getfield(L, opts_index, "truncate");
 			/* Truncate the name if needed. */
 			if (lua_isboolean(L, -1) && lua_toboolean(L, -1) &&
-			    name_len > FIBER_NAME_MAX)
-				name_len = FIBER_NAME_MAX;
+			    name_size > FIBER_NAME_MAX)
+				name_size = FIBER_NAME_MAX;
 			lua_pop(L, 1);
 		}
-		if (name_len > FIBER_NAME_MAX)
+		if (name_size > FIBER_NAME_MAX)
 			luaL_error(L, "Fiber name is too long");
 		fiber_set_name(f, name);
 		return 0;
diff --git a/test/app/fiber.result b/test/app/fiber.result
index 6d9604ad8..7331f61b3 100644
--- a/test/app/fiber.result
+++ b/test/app/fiber.result
@@ -1186,7 +1186,7 @@ fiber = nil
 ---
 ...
 --
--- gh-2622, gh-4011: fiber.name() truncates new name.
+-- gh-2622, gh-4011, gh-4394: fiber.name() truncates new name.
 --
 fiber = require('fiber')
 ---
@@ -1214,14 +1214,14 @@ fiber.name(long_name, {truncate = true})
 ...
 fiber.name()
 ---
-- aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
+- aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
 ...
 f = fiber.self()
 ---
 ...
 fiber.name(f)
 ---
-- aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
+- aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
 ...
 fiber.name(f, 'new_name')
 ---
@@ -1239,7 +1239,7 @@ fiber.name(f, long_name, {truncate = true})
 ...
 fiber.name(f)
 ---
-- aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
+- aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
 ...
 --
 -- gh-3493 fiber.create() does not roll back memtx transaction
diff --git a/test/app/fiber.test.lua b/test/app/fiber.test.lua
index 6df210d9c..b8e9abc6e 100644
--- a/test/app/fiber.test.lua
+++ b/test/app/fiber.test.lua
@@ -504,7 +504,7 @@ test_run:cmd("setopt delimiter ''");
 fiber = nil
 
 --
--- gh-2622, gh-4011: fiber.name() truncates new name.
+-- gh-2622, gh-4011, gh-4394: fiber.name() truncates new name.
 --
 fiber = require('fiber')
 long_name = string.rep('a', 300)
diff --git a/test/unit/fiber.cc b/test/unit/fiber.cc
index 91f7d43f9..b0dfc9b14 100644
--- a/test/unit/fiber.cc
+++ b/test/unit/fiber.cc
@@ -171,9 +171,9 @@ fiber_name_test()
 
 	note("set new fiber name: %s.\n", fiber_name(fiber()));
 
-	const char *long_name = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
-		"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"\
-		"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa";
+	char long_name[FIBER_NAME_MAX + 30];
+	memset(long_name, 'a', sizeof(long_name));
+	long_name[sizeof(long_name) - 1] = 0;
 
 	fiber_set_name(fiber(), long_name);
 
diff --git a/test/unit/fiber.result b/test/unit/fiber.result
index 7c9f85dcd..a61e0a2b8 100644
--- a/test/unit/fiber.result
+++ b/test/unit/fiber.result
@@ -5,7 +5,7 @@ SystemError Failed to allocate 42 bytes in allocator for exception: Cannot alloc
 
 # set new fiber name: Horace.
 
-# fiber name is truncated: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.
+# fiber name is truncated: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.
 
 	*** fiber_name_test: done ***
 	*** fiber_join_test ***

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

* Re: [Tarantool-patches] [PATCH v2 1/1] fiber: extend max fiber name length to 255
  2020-03-16 12:48 ` Nikita Pettik
@ 2020-03-16 22:29   ` Vladislav Shpilevoy
  0 siblings, 0 replies; 10+ messages in thread
From: Vladislav Shpilevoy @ 2020-03-16 22:29 UTC (permalink / raw)
  To: Nikita Pettik; +Cc: tarantool-patches

Indeed. Ask everyone to add them, and forgot myself.

@ChangeLog
- Fiber.name maximal length is extended to 255 (gh-4394).

On 16/03/2020 13:48, Nikita Pettik wrote:
> On 15 Mar 16:11, Vladislav Shpilevoy wrote:
>> Users keep complaining about too short fiber name. New limit is
>> 255, should be enough for any sane name.
>>
>> Closes #4394
>>
>> @TarantoolBot document
>> Title: fiber.name length limit.
>>
>> It was 32, now it is 255. Besides, it seems like `fiber.name`
>> `{truncate = true}` option is not documented.
>>
>> By default, if a new name is too long, `fiber.name(new_name)`
>> fails with an exception. To make it always succeed there is an
>> option 'truncate': `fiber.name(new_name, {truncate = true})`. It
>> truncates the name to the max length if it is too long.
>> ---
>> Branch: http://github.com/tarantool/tarantool/tree/gerold103/gh-4394-fiber-name-inline
>> Issue: https://github.com/tarantool/tarantool/issues/4394
>>
>> Changes in v2:
>> - Short fiber names are kept inside struct fiber.
>>
> 
> LGTM. Seems you forgot to add changelog for github release.
>  
> 

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

* Re: [Tarantool-patches] [PATCH v2 1/1] fiber: extend max fiber name length to 255
  2020-03-16 22:28   ` Vladislav Shpilevoy
@ 2020-03-16 22:31     ` Cyrill Gorcunov
  0 siblings, 0 replies; 10+ messages in thread
From: Cyrill Gorcunov @ 2020-03-16 22:31 UTC (permalink / raw)
  To: Vladislav Shpilevoy; +Cc: tarantool-patches

On Mon, Mar 16, 2020 at 11:28:46PM +0100, Vladislav Shpilevoy wrote:
> Hi! Thanks for the review!
> 
> > Thank, Vlad! I like the patch. There is only one concern I have: for
> > some reason we has been defining faiber name as
> > 
> > char name[FIBER_NAME_MAX + 1];
> > 
> > where FIBER_NAME_MAX = 32 and finally this expands to "char name[33];"
> > I'm too lazy to find who exactly introduced this but it is bloody
> > wrong: compiler alings members to eliminate data access penalty, thus
> > _actually_ it will be defined as 8 multilier, ie 40 bytes.
> > 
> > Thus, if you don't mind I propose make FIBER_NAME_INLINE = 40
> > *including* string terminating zero.
> > 
> > Actually we can make it on top then. Up to you. Anyway
> > 
> > Reviewed-by: Cyrill Gorcunov <gorcunov@gmail.com>
> 
> Yes, good point.
> 
> Here is diff. The whole patch is in the bottom.

Thanks a huge, Vlad! My review tag remains.

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

* Re: [Tarantool-patches] [PATCH v2 1/1] fiber: extend max fiber name length to 255
  2020-03-15 15:11 [Tarantool-patches] [PATCH v2 1/1] fiber: extend max fiber name length to 255 Vladislav Shpilevoy
  2020-03-16  7:19 ` Cyrill Gorcunov
  2020-03-16 12:48 ` Nikita Pettik
@ 2020-03-18 21:35 ` Vladislav Shpilevoy
  2020-03-18 23:57   ` Nikita Pettik
  2 siblings, 1 reply; 10+ messages in thread
From: Vladislav Shpilevoy @ 2020-03-18 21:35 UTC (permalink / raw)
  To: tarantool-patches, gorcunov, n.pettik

Nikita, please, do a second review.

On 15/03/2020 16:11, Vladislav Shpilevoy wrote:
> Users keep complaining about too short fiber name. New limit is
> 255, should be enough for any sane name.
> 
> Closes #4394
> 
> @TarantoolBot document
> Title: fiber.name length limit.
> 
> It was 32, now it is 255. Besides, it seems like `fiber.name`
> `{truncate = true}` option is not documented.
> 
> By default, if a new name is too long, `fiber.name(new_name)`
> fails with an exception. To make it always succeed there is an
> option 'truncate': `fiber.name(new_name, {truncate = true})`. It
> truncates the name to the max length if it is too long.
> ---
> Branch: http://github.com/tarantool/tarantool/tree/gerold103/gh-4394-fiber-name-inline
> Issue: https://github.com/tarantool/tarantool/issues/4394
> 
> Changes in v2:
> - Short fiber names are kept inside struct fiber.
> 
>  src/lib/core/fiber.c    | 28 ++++++++++++++++++++++++++--
>  src/lib/core/fiber.h    | 14 +++++++++++---
>  test/app/fiber.result   |  8 ++++----
>  test/app/fiber.test.lua |  2 +-
>  test/unit/fiber.cc      |  6 +++---
>  test/unit/fiber.result  |  2 +-
>  6 files changed, 46 insertions(+), 14 deletions(-)
> 
> diff --git a/src/lib/core/fiber.c b/src/lib/core/fiber.c
> index ada7972cb..359ef1187 100644
> --- a/src/lib/core/fiber.c
> +++ b/src/lib/core/fiber.c
> @@ -912,8 +912,26 @@ fiber_loop(MAYBE_UNUSED void *data)
>  void
>  fiber_set_name(struct fiber *fiber, const char *name)
>  {
> -	assert(name != NULL);
> -	snprintf(fiber->name, sizeof(fiber->name), "%s", name);
> +	size_t len = strlen(name);
> +	if (len <= FIBER_NAME_INLINE) {
> +		if (fiber->name != fiber->inline_name) {
> +			free(fiber->name);
> +			fiber->name = fiber->inline_name;
> +		}
> +	} else {
> +		if (len > FIBER_NAME_MAX)
> +			len = FIBER_NAME_MAX;
> +		char *new_name;
> +		if (fiber->name != fiber->inline_name)
> +			new_name = realloc(fiber->name, len + 1);
> +		else
> +			new_name = malloc(len + 1);
> +		if (new_name == NULL)
> +			panic("fiber_set_name() failed with OOM");
> +		fiber->name = new_name;
> +	}
> +	memcpy(fiber->name, name, len);
> +	fiber->name[len] = 0;
>  }
>  
>  static inline void *
> @@ -1242,6 +1260,9 @@ fiber_destroy(struct cord *cord, struct fiber *f)
>  	region_destroy(&f->gc);
>  	fiber_stack_destroy(f, &cord->slabc);
>  	diag_destroy(&f->diag);
> +	if (f->name != f->inline_name)
> +		free(f->name);
> +	f->name = NULL;
>  }
>  
>  void
> @@ -1357,6 +1378,7 @@ cord_create(struct cord *cord, const char *name)
>  	fiber_reset(&cord->sched);
>  	diag_create(&cord->sched.diag);
>  	region_create(&cord->sched.gc, &cord->slabc);
> +	cord->sched.name = NULL;
>  	fiber_set_name(&cord->sched, "sched");
>  	cord->fiber = &cord->sched;
>  
> @@ -1405,6 +1427,8 @@ cord_destroy(struct cord *cord)
>  	}
>  	region_destroy(&cord->sched.gc);
>  	diag_destroy(&cord->sched.diag);
> +	if (cord->sched.name != cord->sched.inline_name)
> +		free(cord->sched.name);
>  	slab_cache_destroy(&cord->slabc);
>  }
>  
> diff --git a/src/lib/core/fiber.h b/src/lib/core/fiber.h
> index 8a3e5aef5..fedc37c87 100644
> --- a/src/lib/core/fiber.h
> +++ b/src/lib/core/fiber.h
> @@ -111,7 +111,10 @@ struct cpu_stat {
>  
>  #endif /* ENABLE_FIBER_TOP */
>  
> -enum { FIBER_NAME_MAX = 32 };
> +enum {
> +	FIBER_NAME_INLINE = 32,
> +	FIBER_NAME_MAX = 255
> +};
>  
>  /**
>   * Fiber ids [0; 100] are reserved.
> @@ -510,7 +513,12 @@ struct fiber {
>  	struct ipc_wait_pad *wait_pad;
>  	/** Exception which caused this fiber's death. */
>  	struct diag diag;
> -	char name[FIBER_NAME_MAX + 1];
> +	/**
> +	 * Name points at inline_name in case it is short. Long
> +	 * name is allocated on the heap.
> +	 */
> +	char *name;
> +	char inline_name[FIBER_NAME_INLINE + 1];
>  };
>  
>  /** Invoke on_stop triggers and delete them. */
> @@ -578,7 +586,7 @@ struct cord {
>  	struct slab_cache slabc;
>  	/** The "main" fiber of this cord, the scheduler. */
>  	struct fiber sched;
> -	char name[FIBER_NAME_MAX];
> +	char name[FIBER_NAME_INLINE + 1];
>  };
>  
>  extern __thread struct cord *cord_ptr;
> diff --git a/test/app/fiber.result b/test/app/fiber.result
> index 6d9604ad8..7331f61b3 100644
> --- a/test/app/fiber.result
> +++ b/test/app/fiber.result
> @@ -1186,7 +1186,7 @@ fiber = nil
>  ---
>  ...
>  --
> --- gh-2622, gh-4011: fiber.name() truncates new name.
> +-- gh-2622, gh-4011, gh-4394: fiber.name() truncates new name.
>  --
>  fiber = require('fiber')
>  ---
> @@ -1214,14 +1214,14 @@ fiber.name(long_name, {truncate = true})
>  ...
>  fiber.name()
>  ---
> -- aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
> +- aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
>  ...
>  f = fiber.self()
>  ---
>  ...
>  fiber.name(f)
>  ---
> -- aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
> +- aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
>  ...
>  fiber.name(f, 'new_name')
>  ---
> @@ -1239,7 +1239,7 @@ fiber.name(f, long_name, {truncate = true})
>  ...
>  fiber.name(f)
>  ---
> -- aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
> +- aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
>  ...
>  --
>  -- gh-3493 fiber.create() does not roll back memtx transaction
> diff --git a/test/app/fiber.test.lua b/test/app/fiber.test.lua
> index 6df210d9c..b8e9abc6e 100644
> --- a/test/app/fiber.test.lua
> +++ b/test/app/fiber.test.lua
> @@ -504,7 +504,7 @@ test_run:cmd("setopt delimiter ''");
>  fiber = nil
>  
>  --
> --- gh-2622, gh-4011: fiber.name() truncates new name.
> +-- gh-2622, gh-4011, gh-4394: fiber.name() truncates new name.
>  --
>  fiber = require('fiber')
>  long_name = string.rep('a', 300)
> diff --git a/test/unit/fiber.cc b/test/unit/fiber.cc
> index 91f7d43f9..b0dfc9b14 100644
> --- a/test/unit/fiber.cc
> +++ b/test/unit/fiber.cc
> @@ -171,9 +171,9 @@ fiber_name_test()
>  
>  	note("set new fiber name: %s.\n", fiber_name(fiber()));
>  
> -	const char *long_name = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
> -		"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"\
> -		"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa";
> +	char long_name[FIBER_NAME_MAX + 30];
> +	memset(long_name, 'a', sizeof(long_name));
> +	long_name[sizeof(long_name) - 1] = 0;
>  
>  	fiber_set_name(fiber(), long_name);
>  
> diff --git a/test/unit/fiber.result b/test/unit/fiber.result
> index 7c9f85dcd..a61e0a2b8 100644
> --- a/test/unit/fiber.result
> +++ b/test/unit/fiber.result
> @@ -5,7 +5,7 @@ SystemError Failed to allocate 42 bytes in allocator for exception: Cannot alloc
>  
>  # set new fiber name: Horace.
>  
> -# fiber name is truncated: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.
> +# fiber name is truncated: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.
>  
>  	*** fiber_name_test: done ***
>  	*** fiber_join_test ***
> 

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

* Re: [Tarantool-patches] [PATCH v2 1/1] fiber: extend max fiber name length to 255
  2020-03-18 21:35 ` Vladislav Shpilevoy
@ 2020-03-18 23:57   ` Nikita Pettik
  2020-03-19 22:30     ` Vladislav Shpilevoy
  0 siblings, 1 reply; 10+ messages in thread
From: Nikita Pettik @ 2020-03-18 23:57 UTC (permalink / raw)
  To: Vladislav Shpilevoy; +Cc: tarantool-patches

On 18 Mar 22:35, Vladislav Shpilevoy wrote:
> Nikita, please, do a second review.

I'm okay with this patch (https://lists.tarantool.org/pipermail/tarantool-patches/2020-March/014824.html)
Will push tomorrow if nobody has objections. 

> On 15/03/2020 16:11, Vladislav Shpilevoy wrote:
> > Users keep complaining about too short fiber name. New limit is
> > 255, should be enough for any sane name.
> > 
> > Closes #4394
> > 
> > @TarantoolBot document
> > Title: fiber.name length limit.
> > 
> > It was 32, now it is 255. Besides, it seems like `fiber.name`
> > `{truncate = true}` option is not documented.
> > 
> > By default, if a new name is too long, `fiber.name(new_name)`
> > fails with an exception. To make it always succeed there is an
> > option 'truncate': `fiber.name(new_name, {truncate = true})`. It
> > truncates the name to the max length if it is too long.
> > ---
> > Branch: http://github.com/tarantool/tarantool/tree/gerold103/gh-4394-fiber-name-inline
> > Issue: https://github.com/tarantool/tarantool/issues/4394
> > 
> > Changes in v2:
> > - Short fiber names are kept inside struct fiber.
> > 
> >  src/lib/core/fiber.c    | 28 ++++++++++++++++++++++++++--
> >  src/lib/core/fiber.h    | 14 +++++++++++---
> >  test/app/fiber.result   |  8 ++++----
> >  test/app/fiber.test.lua |  2 +-
> >  test/unit/fiber.cc      |  6 +++---
> >  test/unit/fiber.result  |  2 +-
> >  6 files changed, 46 insertions(+), 14 deletions(-)
> > 
> > diff --git a/src/lib/core/fiber.c b/src/lib/core/fiber.c
> > index ada7972cb..359ef1187 100644
> > --- a/src/lib/core/fiber.c
> > +++ b/src/lib/core/fiber.c
> > @@ -912,8 +912,26 @@ fiber_loop(MAYBE_UNUSED void *data)
> >  void
> >  fiber_set_name(struct fiber *fiber, const char *name)
> >  {
> > -	assert(name != NULL);
> > -	snprintf(fiber->name, sizeof(fiber->name), "%s", name);
> > +	size_t len = strlen(name);
> > +	if (len <= FIBER_NAME_INLINE) {
> > +		if (fiber->name != fiber->inline_name) {
> > +			free(fiber->name);
> > +			fiber->name = fiber->inline_name;
> > +		}
> > +	} else {
> > +		if (len > FIBER_NAME_MAX)
> > +			len = FIBER_NAME_MAX;
> > +		char *new_name;
> > +		if (fiber->name != fiber->inline_name)
> > +			new_name = realloc(fiber->name, len + 1);
> > +		else
> > +			new_name = malloc(len + 1);
> > +		if (new_name == NULL)
> > +			panic("fiber_set_name() failed with OOM");
> > +		fiber->name = new_name;
> > +	}
> > +	memcpy(fiber->name, name, len);
> > +	fiber->name[len] = 0;
> >  }
> >  
> >  static inline void *
> > @@ -1242,6 +1260,9 @@ fiber_destroy(struct cord *cord, struct fiber *f)
> >  	region_destroy(&f->gc);
> >  	fiber_stack_destroy(f, &cord->slabc);
> >  	diag_destroy(&f->diag);
> > +	if (f->name != f->inline_name)
> > +		free(f->name);
> > +	f->name = NULL;
> >  }
> >  
> >  void
> > @@ -1357,6 +1378,7 @@ cord_create(struct cord *cord, const char *name)
> >  	fiber_reset(&cord->sched);
> >  	diag_create(&cord->sched.diag);
> >  	region_create(&cord->sched.gc, &cord->slabc);
> > +	cord->sched.name = NULL;
> >  	fiber_set_name(&cord->sched, "sched");
> >  	cord->fiber = &cord->sched;
> >  
> > @@ -1405,6 +1427,8 @@ cord_destroy(struct cord *cord)
> >  	}
> >  	region_destroy(&cord->sched.gc);
> >  	diag_destroy(&cord->sched.diag);
> > +	if (cord->sched.name != cord->sched.inline_name)
> > +		free(cord->sched.name);
> >  	slab_cache_destroy(&cord->slabc);
> >  }
> >  
> > diff --git a/src/lib/core/fiber.h b/src/lib/core/fiber.h
> > index 8a3e5aef5..fedc37c87 100644
> > --- a/src/lib/core/fiber.h
> > +++ b/src/lib/core/fiber.h
> > @@ -111,7 +111,10 @@ struct cpu_stat {
> >  
> >  #endif /* ENABLE_FIBER_TOP */
> >  
> > -enum { FIBER_NAME_MAX = 32 };
> > +enum {
> > +	FIBER_NAME_INLINE = 32,
> > +	FIBER_NAME_MAX = 255
> > +};
> >  
> >  /**
> >   * Fiber ids [0; 100] are reserved.
> > @@ -510,7 +513,12 @@ struct fiber {
> >  	struct ipc_wait_pad *wait_pad;
> >  	/** Exception which caused this fiber's death. */
> >  	struct diag diag;
> > -	char name[FIBER_NAME_MAX + 1];
> > +	/**
> > +	 * Name points at inline_name in case it is short. Long
> > +	 * name is allocated on the heap.
> > +	 */
> > +	char *name;
> > +	char inline_name[FIBER_NAME_INLINE + 1];
> >  };
> >  
> >  /** Invoke on_stop triggers and delete them. */
> > @@ -578,7 +586,7 @@ struct cord {
> >  	struct slab_cache slabc;
> >  	/** The "main" fiber of this cord, the scheduler. */
> >  	struct fiber sched;
> > -	char name[FIBER_NAME_MAX];
> > +	char name[FIBER_NAME_INLINE + 1];
> >  };
> >  
> >  extern __thread struct cord *cord_ptr;
> > diff --git a/test/app/fiber.result b/test/app/fiber.result
> > index 6d9604ad8..7331f61b3 100644
> > --- a/test/app/fiber.result
> > +++ b/test/app/fiber.result
> > @@ -1186,7 +1186,7 @@ fiber = nil
> >  ---
> >  ...
> >  --
> > --- gh-2622, gh-4011: fiber.name() truncates new name.
> > +-- gh-2622, gh-4011, gh-4394: fiber.name() truncates new name.
> >  --
> >  fiber = require('fiber')
> >  ---
> > @@ -1214,14 +1214,14 @@ fiber.name(long_name, {truncate = true})
> >  ...
> >  fiber.name()
> >  ---
> > -- aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
> > +- aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
> >  ...
> >  f = fiber.self()
> >  ---
> >  ...
> >  fiber.name(f)
> >  ---
> > -- aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
> > +- aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
> >  ...
> >  fiber.name(f, 'new_name')
> >  ---
> > @@ -1239,7 +1239,7 @@ fiber.name(f, long_name, {truncate = true})
> >  ...
> >  fiber.name(f)
> >  ---
> > -- aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
> > +- aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
> >  ...
> >  --
> >  -- gh-3493 fiber.create() does not roll back memtx transaction
> > diff --git a/test/app/fiber.test.lua b/test/app/fiber.test.lua
> > index 6df210d9c..b8e9abc6e 100644
> > --- a/test/app/fiber.test.lua
> > +++ b/test/app/fiber.test.lua
> > @@ -504,7 +504,7 @@ test_run:cmd("setopt delimiter ''");
> >  fiber = nil
> >  
> >  --
> > --- gh-2622, gh-4011: fiber.name() truncates new name.
> > +-- gh-2622, gh-4011, gh-4394: fiber.name() truncates new name.
> >  --
> >  fiber = require('fiber')
> >  long_name = string.rep('a', 300)
> > diff --git a/test/unit/fiber.cc b/test/unit/fiber.cc
> > index 91f7d43f9..b0dfc9b14 100644
> > --- a/test/unit/fiber.cc
> > +++ b/test/unit/fiber.cc
> > @@ -171,9 +171,9 @@ fiber_name_test()
> >  
> >  	note("set new fiber name: %s.\n", fiber_name(fiber()));
> >  
> > -	const char *long_name = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
> > -		"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"\
> > -		"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa";
> > +	char long_name[FIBER_NAME_MAX + 30];
> > +	memset(long_name, 'a', sizeof(long_name));
> > +	long_name[sizeof(long_name) - 1] = 0;
> >  
> >  	fiber_set_name(fiber(), long_name);
> >  
> > diff --git a/test/unit/fiber.result b/test/unit/fiber.result
> > index 7c9f85dcd..a61e0a2b8 100644
> > --- a/test/unit/fiber.result
> > +++ b/test/unit/fiber.result
> > @@ -5,7 +5,7 @@ SystemError Failed to allocate 42 bytes in allocator for exception: Cannot alloc
> >  
> >  # set new fiber name: Horace.
> >  
> > -# fiber name is truncated: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.
> > +# fiber name is truncated: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.
> >  
> >  	*** fiber_name_test: done ***
> >  	*** fiber_join_test ***
> > 

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

* Re: [Tarantool-patches] [PATCH v2 1/1] fiber: extend max fiber name length to 255
  2020-03-18 23:57   ` Nikita Pettik
@ 2020-03-19 22:30     ` Vladislav Shpilevoy
  2020-03-20  1:59       ` Nikita Pettik
  0 siblings, 1 reply; 10+ messages in thread
From: Vladislav Shpilevoy @ 2020-03-19 22:30 UTC (permalink / raw)
  To: Nikita Pettik; +Cc: tarantool-patches

Hi! Shame on me, I forgot to update the branch. Now it is
done.

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

* Re: [Tarantool-patches] [PATCH v2 1/1] fiber: extend max fiber name length to 255
  2020-03-19 22:30     ` Vladislav Shpilevoy
@ 2020-03-20  1:59       ` Nikita Pettik
  0 siblings, 0 replies; 10+ messages in thread
From: Nikita Pettik @ 2020-03-20  1:59 UTC (permalink / raw)
  To: Vladislav Shpilevoy; +Cc: tarantool-patches

On 19 Mar 23:30, Vladislav Shpilevoy wrote:
> Hi! Shame on me, I forgot to update the branch. Now it is
> done.

Pushed to master, updated changelog correspondingly. Branches
(containing both patch versions) are dropped. Thanks.

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

end of thread, other threads:[~2020-03-20  1:59 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-15 15:11 [Tarantool-patches] [PATCH v2 1/1] fiber: extend max fiber name length to 255 Vladislav Shpilevoy
2020-03-16  7:19 ` Cyrill Gorcunov
2020-03-16 22:28   ` Vladislav Shpilevoy
2020-03-16 22:31     ` Cyrill Gorcunov
2020-03-16 12:48 ` Nikita Pettik
2020-03-16 22:29   ` Vladislav Shpilevoy
2020-03-18 21:35 ` Vladislav Shpilevoy
2020-03-18 23:57   ` Nikita Pettik
2020-03-19 22:30     ` Vladislav Shpilevoy
2020-03-20  1:59       ` Nikita Pettik

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