Re: [Tarantool-patches] [PATCH 11/20] net.box: rewrite response decoder in C

[Vladimir Davydov via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

On Tue, Jul 27, 2021 at 05:07:42PM +0300, Cyrill Gorcunov via Tarantool-patches wrote:
> On Fri, Jul 23, 2021 at 02:07:21PM +0300, Vladimir Davydov via Tarantool-patches wrote:
> > This patch moves method_decoder table from Lua to C. This is a step
> > towards rewriting performance-critical parts of net.box in C.
> ...
> > +static int
> > +netbox_decode_method(struct lua_State *L)
> > +{
> > +	typedef void (*method_decoder_f)(struct lua_State *L, const char **data,
> > +					 const char *data_end,
> > +					 struct tuple_format *format);
> > +	static method_decoder_f method_decoder[] = {
> > +		[NETBOX_PING]		= netbox_decode_nil,
> > +		[NETBOX_CALL_16]	= netbox_decode_select,
> ...
> > +		[NETBOX_INJECT]		= netbox_decode_table,
> > +	};
> > +	enum netbox_method method = lua_tointeger(L, 1);
> > +	assert(method < netbox_method_MAX);
> 
> Should not it be runtime testing like
> 
> 	if (method < 0 || method >= lengthof(method_decoder) ||
> 	    method_decoder[method] == NULL) {
> 		// Some Lua error thrown
> 		return luaT_error();
> 	}
> 
> or we've validated this data somewhere already?

This is an internal function, which is only called by net_box.lua.
All these constants are internal, as well. So I don't think it's
worth validating input here or anywhere else in net_box.c.