[Tarantool-patches] [PATCH 2.X 6/7] module api: luaL_checkibuf & luaL_checkconstchar

[Vladislav Shpilevoy]

Thanks for the patch!

See 2 comments below.

> diff --git a/src/lua/utils.h b/src/lua/utils.h
> index da0140076..6b10d2755 100644
> --- a/src/lua/utils.h
> +++ b/src/lua/utils.h
> @@ -589,6 +589,23 @@ luaT_temp_luastate(int *coro_ref, int *top);
>  void
>  luaT_release_temp_luastate(struct lua_State *L, int coro_ref, int top);
>  
> +/**
> + * Check if a value on @a L stack by index @a idx is an ibuf
> + * object. Both 'struct ibuf' and 'struct ibuf *' are accepted.
> + * Returns NULL, if can't convert - not an ibuf object.
> + */
> +struct ibuf *
> +luaL_checkibuf(struct lua_State *L, int idx);

1. IMO 'check' is almost always worse than 'is'. Because you leave
a user no choice but to use lua_cpcall if he does not want an
exception. With 'is' he would be able to decide whether he wants to
throw. The same for the method below.

Also what exactly is the reason you need the ibuf checking for?
Ibuf is not exposed as a part of tarantool binary. It is a part of
small library. When we want to export parts of small from inside
of the executable, we need to obfuscate the types and wrap the
functions, because user's small library may be different from the
executable's small.

If you use ibuf internally in the merger, what is wrong with
registering it in the merger, and storing a global variable of
uint32_t type, like we do inside tarantool executable? You will even
get the same type index, but won't need to carry small library
types into the public lua API.

> +
> +/**
> + * Check if a value on @a L stack by index @a idx is pointer at
> + * char or const char. '(char *)NULL' is also considered a valid
> + * char pointer.
> + */
> +int
> +luaL_checkconstchar(struct lua_State *L, int idx, const char **res,
> +		    uint32_t *cdata_type_p);

2. What is 'res'? What is 'cdata_type_p'?

> +
>  /** \endcond public */
>  
>  /**