From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id 264106F3E5; Mon, 8 Nov 2021 18:15:33 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 264106F3E5 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1636384533; bh=Z/Q94FQkaPxyNbW4oeg/zZ4Jtx5joQpieiDT1mLHq9I=; h=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=mIEbzEIALezEbDHoJso9hcFVGP/G6YV0DM6aOq6/h26y6Iow4wXfubCXjlBqjLOF0 JkvejHUR2pOs9CWU5IRt/5oJI2Yx3hxz8/eyRwt9vopSWYyZmU9WkmtCOUsmvW/vH+ zXnigMmaxC9kibac2vmM5fyMNwBH2gUGSAVDPsDA= Received: from smtp42.i.mail.ru (smtp42.i.mail.ru [94.100.177.102]) (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 A4AE56F3E5 for ; Mon, 8 Nov 2021 18:15:30 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org A4AE56F3E5 Received: by smtp42.i.mail.ru with esmtpa (envelope-from ) id 1mk6Mr-0001Dz-H8; Mon, 08 Nov 2021 18:15:30 +0300 Message-ID: <7b58d78b-2a74-71c3-6bd1-094746c73010@tarantool.org> Date: Mon, 8 Nov 2021 18:15:29 +0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0) Gecko/20100101 Thunderbird/91.2.1 To: Vladislav Shpilevoy , tarantool-patches@dev.tarantool.org, vdavydov@tarantool.org References: <375c2c4037f0517fa0801a752266530151544329.1636156453.git.v.shpilevoy@tarantool.org> Content-Language: en-GB In-Reply-To: <375c2c4037f0517fa0801a752266530151544329.1636156453.git.v.shpilevoy@tarantool.org> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-4EC0790: 10 X-7564579A: B8F34718100C35BD X-77F55803: 4F1203BC0FB41BD9731B3922EC063979B64913EBF93CACC62CE6D1034D81877200894C459B0CD1B950DF6FA384AD7C1EB877379A266C37C7AA1D1AB37C96470E40D08F30CB48EA5E X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE792C68BF9CD4C0E9EEA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637B1BE0CF094621A628638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D8B6EEC2003B6C0EFF5FB59A296E5EA383117882F4460429724CE54428C33FAD305F5C1EE8F4F765FCAA867293B0326636D2E47CDBA5A96583BD4B6F7A4D31EC0BC014FD901B82EE079FA2833FD35BB23D27C277FBC8AE2E8BAA867293B0326636D2E47CDBA5A96583BA9C0B312567BB2376E601842F6C81A19E625A9149C048EE7B96B19DC4093321B01F8B705E65F4D2D8FC6C240DEA7642DBF02ECDB25306B2B78CF848AE20165D0A6AB1C7CE11FEE362B3BD3CC35DA588C0837EA9F3D19764C4224003CC836476EA7A3FFF5B025636E2021AF6380DFAD1A18204E546F3947CB11811A4A51E3B096D1867E19FE1407959CC434672EE6371089D37D7C0E48F6C8AA50765F7900637C3090DF2C0002BD1EFF80C71ABB335746BA297DBC24807EABDAD6C7F3747799A X-B7AD71C0: AC4F5C86D027EB782CDD5689AFBDA7A213B5FB47DCBC3458834459D11680B5050C315A302238E54BF229F88B7EDBBC0D X-C1DE0DAB: 0D63561A33F958A5FC1532D5549FC0B22312BBD479BA13126B47CF095C6FDE74D59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA753753CEE10E4ED4A7410CA545F18667F91A7EA1CDA0B5A7A0 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D3454548929AF40B4286C7771D490C5F062FD30914FEC68C913D241BE89C13B7ECF2A3B4CA31AC4F7A21D7E09C32AA3244C85FF5282567075D945951CBD865621033C6EB905E3A8056BFACE5A9C96DEB163 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2bioj9/1IdNVT2RYwPkTnz2WZoA== X-Mailru-Sender: 11C2EC085EDE56FA38FD4C59F7EFE40733B5C6D1893C2B708DBC07BBD47590064FC44C16C84DF5F46BB2E709EA627F343C7DDD459B58856F0E45BC603594F5A135B915D4279FF0579437F6177E88F7363CDA0F3B3F5B9367 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH 3/9] error: move code to struct error from ClientError X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Serge Petrenko via Tarantool-patches Reply-To: Serge Petrenko Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" 06.11.2021 02:56, Vladislav Shpilevoy пишет: > All optional fields soon will be moved into error_payload. Code > was optional too. But it is needed too often, the most used field. > The patch moves it into struct error to make it more accessible. > > Also in future it should allow to drop the hack > ClientError::get_errcode() which tries to return error code > depending on error type. But could just store the code right away. > > As a consequence of the patch, errors which didn't have an error > code at all before, such as LuajitError, now have it 0 in Lua. > > Part of #5568 Thanks for the patch! Please, find 2 minor comments below. > --- > src/box/error.cc | 15 +++++++-------- > src/box/error.h | 4 +--- > src/box/index.cc | 4 ++-- > src/box/mp_error.cc | 7 +++---- > src/lib/core/diag.c | 1 + > src/lib/core/diag.h | 2 ++ > src/lua/error.lua | 3 ++- > test/engine/func_index.result | 3 ++- > test/unit/mp_error.cc | 2 +- > 9 files changed, 21 insertions(+), 20 deletions(-) > > diff --git a/src/box/error.cc b/src/box/error.cc > index 225b377b7..bc0d46601 100644 > --- a/src/box/error.cc > +++ b/src/box/error.cc > @@ -76,7 +76,7 @@ box_error_set(const char *file, unsigned line, uint32_t code, > struct error *e = BuildClientError(file, line, ER_UNKNOWN); > ClientError *client_error = type_cast(ClientError, e); > if (client_error) { > - client_error->m_errcode = code; > + client_error->code = code; > va_list ap; > va_start(ap, fmt); > error_vformat_msg(e, fmt, ap); > @@ -94,7 +94,7 @@ box_error_new_va(const char *file, unsigned line, uint32_t code, > struct error *e = BuildClientError(file, line, ER_UNKNOWN); > ClientError *client_error = type_cast(ClientError, e); > if (client_error != NULL) { > - client_error->m_errcode = code; > + client_error->code = code; > error_vformat_msg(e, fmt, ap); > } > return e; > @@ -170,7 +170,7 @@ ClientError::ClientError(const type_info *type, const char *file, unsigned line, > uint32_t errcode) > :Exception(type, file, line) > { > - m_errcode = errcode; > + code = errcode; > if (rmean_error) > rmean_collect(rmean_error, RMEAN_ERROR, 1); > } > @@ -181,7 +181,7 @@ ClientError::ClientError(const char *file, unsigned line, > { > va_list ap; > va_start(ap, errcode); > - error_vformat_msg(this, tnt_errcode_desc(m_errcode), ap); > + error_vformat_msg(this, tnt_errcode_desc(errcode), ap); > va_end(ap); > } > > @@ -194,7 +194,7 @@ BuildClientError(const char *file, unsigned line, uint32_t errcode, ...) > va_start(ap, errcode); > error_vformat_msg(e, tnt_errcode_desc(errcode), ap); > va_end(ap); > - e->m_errcode = errcode; > + e->code = errcode; > return e; > } catch (OutOfMemory *e) { > return e; > @@ -204,8 +204,7 @@ BuildClientError(const char *file, unsigned line, uint32_t errcode, ...) > void > ClientError::log() const > { > - say_file_line(S_ERROR, file, line, errmsg, "%s", > - tnt_errcode_str(m_errcode)); > + say_file_line(S_ERROR, file, line, errmsg, "%s", tnt_errcode_str(code)); > } > > > @@ -296,7 +295,7 @@ AccessDeniedError::AccessDeniedError(const char *file, unsigned int line, > bool run_trigers) > :ClientError(&type_AccessDeniedError, file, line, ER_ACCESS_DENIED) > { > - error_format_msg(this, tnt_errcode_desc(m_errcode), > + error_format_msg(this, tnt_errcode_desc(code), > access_type, object_type, object_name, user_name); > > struct on_access_denied_ctx ctx = {access_type, object_type, object_name}; > diff --git a/src/box/error.h b/src/box/error.h > index 338121dd9..30ab36a97 100644 > --- a/src/box/error.h > +++ b/src/box/error.h > @@ -208,14 +208,12 @@ public: > int > errcode() const > { > - return m_errcode; > + return code; > } > > ClientError(const char *file, unsigned line, uint32_t errcode, ...); > > static uint32_t get_errcode(const struct error *e); > - /* client errno code */ > - int m_errcode; > protected: > ClientError(const type_info *type, const char *file, unsigned line, > uint32_t errcode); > diff --git a/src/box/index.cc b/src/box/index.cc > index 0651868d3..be14827be 100644 > --- a/src/box/index.cc > +++ b/src/box/index.cc > @@ -47,8 +47,8 @@ UnsupportedIndexFeature::UnsupportedIndexFeature(const char *file, > : ClientError(file, line, ER_UNKNOWN) > { > struct space *space = space_cache_find_xc(index_def->space_id); > - m_errcode = ER_UNSUPPORTED_INDEX_FEATURE; > - error_format_msg(this, tnt_errcode_desc(m_errcode), index_def->name, > + code = ER_UNSUPPORTED_INDEX_FEATURE; > + error_format_msg(this, tnt_errcode_desc(code), index_def->name, > index_type_strs[index_def->type], > space->def->name, space->def->engine_name, what); > } > diff --git a/src/box/mp_error.cc b/src/box/mp_error.cc > index 059f9bc85..35c770400 100644 > --- a/src/box/mp_error.cc > +++ b/src/box/mp_error.cc > @@ -261,10 +261,8 @@ missing_fields: > } > > if (strcmp(mp_error->type, "ClientError") == 0) { > - ClientError *e = new ClientError(mp_error->file, mp_error->line, > - ER_UNKNOWN); > - e->m_errcode = mp_error->code; > - err = e; > + err = new ClientError(mp_error->file, mp_error->line, > + ER_UNKNOWN); > } else if (strcmp(mp_error->type, "CustomError") == 0) { > if (mp_error->custom_type == NULL) > goto missing_fields; > @@ -320,6 +318,7 @@ missing_fields: > err = new ClientError(mp_error->file, mp_error->line, > ER_UNKNOWN); > } > + err->code = mp_error->code; > err->saved_errno = mp_error->saved_errno; > error_format_msg(err, "%s", mp_error->message); > return err; > diff --git a/src/lib/core/diag.c b/src/lib/core/diag.c > index f872be6a7..b557ca667 100644 > --- a/src/lib/core/diag.c > +++ b/src/lib/core/diag.c > @@ -116,6 +116,7 @@ error_create(struct error *e, > e->type = type; > e->refs = 0; > e->saved_errno = 0; > + e->code = 0; > if (file != NULL) { > snprintf(e->file, sizeof(e->file), "%s", file); > e->line = line; > diff --git a/src/lib/core/diag.h b/src/lib/core/diag.h > index d7d09f6fb..30b8d2f5d 100644 > --- a/src/lib/core/diag.h > +++ b/src/lib/core/diag.h > @@ -87,6 +87,8 @@ struct error { > * to the standard library, then it is 0. > */ > int saved_errno; > + /** Error code. Shortest possible description of error's reason. */ > + int code; > /** Line number. */ > unsigned line; > /* Source file name. */ > diff --git a/src/lua/error.lua b/src/lua/error.lua > index ebc784f07..9fb227df4 100644 > --- a/src/lua/error.lua > +++ b/src/lua/error.lua > @@ -18,6 +18,7 @@ struct error { > const struct type_info *_type; > int64_t _refs; > int _saved_errno; > + int code; 1. All the other fields are prefixed with "_" for some reason.    Let's do the same for "code", just to comply. > /** Line number. */ > unsigned _line; > /* Source file name. */ > @@ -158,7 +159,7 @@ local function error_unpack(err) > if not ffi.istype('struct error', err) then > error("Usage: error:unpack()") > end > - local result = {} > + local result = {code = err.code} 2. Why not add 'code' to 'error_fields' ? > for key, getter in pairs(error_fields) do > result[key] = getter(err) > end > diff --git a/test/engine/func_index.result b/test/engine/func_index.result > index b689c68ec..d3f44f7be 100644 > --- a/test/engine/func_index.result > +++ b/test/engine/func_index.result > @@ -292,7 +292,8 @@ e = e.prev > | ... > e:unpack() > | --- > - | - base_type: LuajitError > + | - code: 0 > + | base_type: LuajitError > | type: LuajitError > | message: '[string "return function(tuple) local ..."]:1: attempt > | to call global ''require'' (a nil value)' > diff --git a/test/unit/mp_error.cc b/test/unit/mp_error.cc > index e85d282e7..fe0cd49b9 100644 > --- a/test/unit/mp_error.cc > +++ b/test/unit/mp_error.cc > @@ -349,7 +349,7 @@ test_decode_unknown_type() > const char *pos = buffer; > struct error *unpacked = error_unpack(&pos, len); > error_ref(unpacked); > - err.code = 0; > + err.code = 1; > err.type = "ClientError"; > ok(error_is_eq_mp_error(unpacked, &err), "check SomeNewError"); > error_unref(unpacked); -- Serge Petrenko