Re: [Tarantool-patches] [PATCH v2 1/3] merger: fix NULL dereference when called via iproto

[Alexander Turenko <alexander.turenko@tarantool.org>]

> >  static int
> > -luaL_merge_source_buffer_fetch(struct merge_source_buffer *source)
> > +luaL_merge_source_buffer_fetch_impl(struct lua_State *L,
> > +				    struct merge_source_buffer *source)
> 
> I see my comment to the former first commit was missed somewhy, so I say
> it here:

Sorry, I misread it a bit and understood as part of illustration that
there are still no consistency even after dropping luaL prefix.

> 
>     - If a function takes lua_State, it is not mean it should get
>       luaL_ prefix. Since the first commit was dropped, this comment
>       we can't fix.
> 
>     - If a function is a method of struct, struct pointer goes *always*
>       first, like 'this' argument in C++. Present of lua_State does
>       not mean it should be first. This can and should be fixed, I think.
> 
> This function in the first place is a method of merge_source_buffer.
> So it should take this argument first. luaL_merge_source_buffer_fetch_impl()
> currently is the only method of merge_source_buffer, which takes
> merge_source_buffer not in the first argument. It means, I don't see
> how the consistency is improved here. The same for
> luaL_merge_source_table_fetch, luaL_merge_source_table_next_impl,
> luaL_merge_source_tuple_fetch, luaL_merge_source_tuple_next_impl.

Moved <struct lua_State *> to the end of an argument list.

Removed the following paragraph from the commit message:

 | Changed order of luaL_merge_source_tuple_fetch() arguments to unify it
 | with other functions (<struct lua_State *>, <struct merge_source *>).