From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp41.i.mail.ru (smtp41.i.mail.ru [94.100.177.101]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id C7641441841 for ; Thu, 26 Mar 2020 04:03:47 +0300 (MSK) Date: Thu, 26 Mar 2020 01:03:47 +0000 From: Nikita Pettik Message-ID: <20200326010347.GB3155@tarantool.org> References: <20200325010231.GA30598@tarantool.org> <32acd313-8d54-5496-55e2-0613fe4bd883@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <32acd313-8d54-5496-55e2-0613fe4bd883@tarantool.org> Subject: Re: [Tarantool-patches] [PATCH 3/7] box/error: don't set error created via box.error.new to diag 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 26 Mar 01:22, Vladislav Shpilevoy wrote: > On 25/03/2020 02:02, Nikita Pettik wrote: > > On 22 Feb 18:18, Vladislav Shpilevoy wrote: > >> Thanks for the patch! > >> > >>> diff --git a/src/box/error.h b/src/box/error.h > >>> index b8c7cf73d..42043ef80 100644 > >>> --- a/src/box/error.h > >>> +++ b/src/box/error.h > >>> @@ -129,6 +129,14 @@ int > >>> box_error_set(const char *file, unsigned line, uint32_t code, > >>> const char *format, ...); > >>> > >>> +/** > >>> + * Construct error object without setting it in the diagnostics > >>> + * area. On the memory allocation fail returns NULL. > >>> + */ > >>> +struct error * > >>> +box_error_construct(const char *file, unsigned line, uint32_t code, > >>> + const char *fmt, ...); > >> > >> 5. You added the method to the public C API (it is inside 'cond public' > >> comments). I don't think we should do that. At least until someone > >> asked so. > >> > >> Also why not box_error_new()? 'Construct' seems to be a very strange name. > > > > IMHO _new 'suffix' implies allocating new object, meanwhile _construct - > > filling in already allocated object. It was my understanding. > Exactly. And box_error_construct() allocates a new object. You even said > that in the comment. So where am I wrong? Indeed. I should have paid more attention. Let's rename it to _new(). Otherwise patch looks good to you? > Btw, for initialization of an object in-place we use '_create'. > '_init' is used for initialization of a module. But '_construct' - never. Ok, will keep in mind.