[tarantool-patches] Re: [PATCH] Fix premature cdata collecting in luaT_pusherror()

[Konstantin Osipov <kostja@tarantool.org>]

* Vladislav Shpilevoy <v.shpilevoy@tarantool.org> [18/12/08 01:20]:
> 
> 
> On 07/12/2018 21:15, Alexander Turenko wrote:
> > This is follow up of 28c7e667aee9be8c3288597bcc179d9b4e7b4940 to fix
> > luaT_pusherror() itself, not only luaT_error().
> > 
> > Fixes #1955 (again).
> > 
> > It is backport of 480868ff8f304ead2fd03e317902a1e8d41c6248 for 1.10.
> > ---
> > 
> > https://github.com/tarantool/tarantool/issues/1955
> > https://github.com/tarantool/tarantool/tree/Totktonada/gh-1955-fix-luaT_pusherror-1.10
> > 
> >   src/lua/utils.c | 20 ++++++++++----------
> >   1 file changed, 10 insertions(+), 10 deletions(-)
> > 
> > diff --git a/src/lua/utils.c b/src/lua/utils.c
> > index 27772a9d3..b05d16a73 100644
> > --- a/src/lua/utils.c
> > +++ b/src/lua/utils.c
> > @@ -898,12 +898,20 @@ luaL_error_gc(struct lua_State *L)
> >   void
> >   luaT_pusherror(struct lua_State *L, struct error *e)
> >   {
> > +	/*
> > +	 * gh-1955 luaT_pusherror allocates Lua objects, thus it
> 
> Sorry, but usually we do not mention issue numbers in comments.
> Please, reword the comment so as to do not use explicit issue
> number.

I think it's oK to mention issue ## in a changeset doesn't have a test case.

This all comes from future use: if there is no test, it's
difficult to understand what was the patch about without a quick
link to the bugs database.

> > -	/*
> > -	 * gh-1955 luaT_pusherror allocates Lua objects, thus it may trigger
> > -	 * GC. GC may invoke finalizers which are arbitrary Lua code,
> > -	 * potentially invalidating last error object, hence error_ref
> > -	 * below.
> > -	 */

Alexander only moved this comment. The comment itself was added by
Nick Zavaritsky, I think it was appropriate in this case.


-- 
Konstantin Osipov, Moscow, Russia, +7 903 626 22 32
http://tarantool.io - www.twitter.com/kostja_osipov