From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org> To: Nikita Pettik <korablev@tarantool.org>, tarantool-patches@dev.tarantool.org Subject: Re: [Tarantool-patches] [PATCH] box/error: ref error.prev while accessing it Date: Thu, 16 Apr 2020 02:19:01 +0200 [thread overview] Message-ID: <574107d5-cb85-1b08-3abf-c2c53497ab5e@tarantool.org> (raw) In-Reply-To: <3ee69eaa4ff23647e9ae6fe63e2ae77c427ffe01.1586993218.git.korablev@tarantool.org> Hi! Thanks for the patch! See 2 minor comments below. > diff --git a/src/lib/core/diag.h b/src/lib/core/diag.h > index 7a5454d1c..beb6a7528 100644 > --- a/src/lib/core/diag.h > +++ b/src/lib/core/diag.h > @@ -118,25 +118,8 @@ error_ref(struct error *e) > e->refs++; > } > > -static inline void > -error_unref(struct error *e) 1. To reduce diff size and to keep inlining when possible you can remove word 'static', and add extern inline void error_unref(struct error *e); to diag.c. Yeah, exactly 'extern inline', not just 'extern'. I recently discovered that it is a special thing for such cases. > -{ > - assert(e->refs > 0); > - struct error *to_delete = e; > - while (--to_delete->refs == 0) { > - /* Unlink error from lists completely.*/ > - struct error *cause = to_delete->cause; > - assert(to_delete->effect == NULL); > - if (to_delete->cause != NULL) { > - to_delete->cause->effect = NULL; > - to_delete->cause = NULL; > - } > - to_delete->destroy(to_delete); > - if (cause == NULL) > - return; > - to_delete = cause; > - } > -} > +void > +error_unref(struct error *e); > > diff --git a/test/box/error.result b/test/box/error.result > index df6d0ebc0..49e35b942 100644 > --- a/test/box/error.result > +++ b/test/box/error.result > @@ -831,3 +831,51 @@ assert(box.error.last() == e1) > + | ... > +err.prev > + | --- > + | - null > + | ... > +preve > + | --- > + | - '[string "return function(tuple) local json = require(''..."]:1: attempt to call > + | global ''require'' (a nil value)' > + | ... > + > +s:drop() > + | --- > + | ... > +box.schema.func.drop('runtimeerror') 2. Did you check that preve does not leak when GC starts working? You can use setmetatable({}, {__mode = 'v'}) for that, see examples by grepping " __mode = 'v' ".
next prev parent reply other threads:[~2020-04-16 0:19 UTC|newest] Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-04-15 23:36 Nikita Pettik 2020-04-16 0:19 ` Vladislav Shpilevoy [this message] 2020-04-16 12:34 ` Nikita Pettik 2020-04-16 21:17 ` Vladislav Shpilevoy 2020-04-17 20:06 ` Nikita Pettik 2020-04-17 23:58 ` Vladislav Shpilevoy
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=574107d5-cb85-1b08-3abf-c2c53497ab5e@tarantool.org \ --to=v.shpilevoy@tarantool.org \ --cc=korablev@tarantool.org \ --cc=tarantool-patches@dev.tarantool.org \ --subject='Re: [Tarantool-patches] [PATCH] box/error: ref error.prev while accessing it' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox