From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lj1-f193.google.com (mail-lj1-f193.google.com [209.85.208.193]) (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 41B244696C3 for ; Thu, 26 Mar 2020 19:50:52 +0300 (MSK) Received: by mail-lj1-f193.google.com with SMTP id k21so7158597ljh.2 for ; Thu, 26 Mar 2020 09:50:52 -0700 (PDT) Date: Thu, 26 Mar 2020 19:50:49 +0300 From: Konstantin Osipov Message-ID: <20200326165049.GC21134@atlas> References: <1816245c768216733779be5c7420cc60725287ea.1585097339.git.korablev@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1816245c768216733779be5c7420cc60725287ea.1585097339.git.korablev@tarantool.org> Subject: Re: [Tarantool-patches] [PATCH v2 05/10] 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: Nikita Pettik Cc: tarantool-patches@dev.tarantool.org, v.shpilevoy@tarantool.org * Nikita Pettik [20/03/25 09:32]: > To achieve this let's refactor luaT_error_create() to return error > object instead of setting it via box_error_set(). > luaT_error_create() is used both to handle box.error() and > box.error.new() invocations, and box.error() is still expected to set > error to diagnostic area. So, luaT_error_call() which implements > box.error() processing at the end calls diag_set_error(). > It is worth mentioning that net.box module relied on the fact that > box.error.new() set error to diagnostic area: otherwise request errors > don't get to diagnostic area on client side. > I know there was a discussion about construct vs create vs new. Let me remind about approved name pairs: obj_create/obj_destroy - constructor/destructor, without allocation/deallocation. obj_new/obj_delete - constructor/destructor with allocation/deallocation init/free - library initialization/destruction _construct is not in the vocabulary. I'd avoid it. I know this is bikeshed, pls feel free to ignore in scope of this patch. And sorry for jumping in late. > +struct error * > +box_error_construct(const char *file, unsigned line, uint32_t code, > + const char *fmt, ...) > +{ > + struct error *e = BuildClientError(file, line, ER_UNKNOWN); > + ClientError *client_error = type_cast(ClientError, e); > + if (client_error != NULL) { > + client_error->m_errcode = code; > + va_list ap; > + va_start(ap, fmt); > + error_vformat_msg(e, fmt, ap); > + va_end(ap); > + } > + return e; > +} if box_error_new() is already taken by the public api, options are box_client_error_new() or box_build_client_error()? -- Konstantin Osipov, Moscow, Russia