Re: [tarantool-patches] [PATCH v2 2/3] lua: internal function to parse space format

[Vladislav Shpilevoy <v.shpilevoy@tarantool.org>]

Thanks for the patch!

> +
> +static int
> +lbox_tuple_format_new(struct lua_State *L)
> +{
> +	assert(CTID_STRUCT_TUPLE_FORMAT_PTR != 0);
> +	if (lua_gettop(L) == 0)
> +		return lbox_push_tuple_format(L, tuple_format_runtime);
> +	if (lua_gettop(L) > 1 || ! lua_istable(L, 1))
> +		return luaL_error(L, "Usage: box.internal.format_new(format)");
> +	uint32_t count = lua_objlen(L, 1);
> +	if (count == 0)
> +		return lbox_push_tuple_format(L, tuple_format_runtime);
> +	size_t size = count * sizeof(struct field_def);
> +	struct region *region = &fiber()->gc;
> +	size_t region_svp = region_used(region);
> +	struct field_def *fields =
> +		(struct field_def *)region_alloc(region, size);
> +	if (fields == NULL) {
> +		diag_set(OutOfMemory, size, "region_alloc", "fields");
> +		return luaT_error(L);
> +	}
> +	for (uint32_t i = 0; i < count; ++i) {
> +		size_t len;
> +
> +		fields[i] = field_def_default;
> +
> +		lua_pushinteger(L, i + 1);
> +		lua_gettable(L, 1);
> +
> +		lua_pushstring(L, "type");
> +		lua_gettable(L, -2);
> +		if (! lua_isnil(L, -1)) {
> +			const char *type_name = lua_tolstring(L, -1, &len);
> +			fields[i].type = field_type_by_name(type_name, len);
> +			if (fields[i].type == field_type_MAX) {
> +				const char *err =
> +					"field %d has unknown field type";
> +				return luaL_error(L, tt_sprintf(err, i + 1));

Here and in all other 'return on error' the region leaks. But there is
another point. How can there be errors, if the method is internal? If I
had implemented that patch, I would panic/assert on any error here, except
region OOM.