From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lf1-f68.google.com (mail-lf1-f68.google.com [209.85.167.68]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id B530A469719 for ; Mon, 16 Mar 2020 10:19:30 +0300 (MSK) Received: by mail-lf1-f68.google.com with SMTP id n20so9721580lfl.10 for ; Mon, 16 Mar 2020 00:19:30 -0700 (PDT) Date: Mon, 16 Mar 2020 10:19:27 +0300 From: Cyrill Gorcunov Message-ID: <20200316071927.GZ27301@uranus> References: <7720185c4456a63f0d31bfbe11cdb101e864a3c4.1584284911.git.v.shpilevoy@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <7720185c4456a63f0d31bfbe11cdb101e864a3c4.1584284911.git.v.shpilevoy@tarantool.org> Subject: Re: [Tarantool-patches] [PATCH v2 1/1] fiber: extend max fiber name length to 255 List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Vladislav Shpilevoy Cc: tarantool-patches@dev.tarantool.org 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