Re: [Tarantool-patches] [PATCH v1 1/2] sql: ignore \0 in string passed to C-function

[Vladislav Shpilevoy via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Hi! Thanks for the patch!

See 4 comments below.

On 30.03.2021 13:21, Mergen Imeev via Tarantool-patches wrote:
> Prior to this patch string passed to user-defined C-function from SQL
> was cropped in case it contains '\0'. At the same time, it wasn't
> cropped if it is passed to the function from BOX. Now it isn't cropped
> when passed from SQL.
> 
> Part of #5938
> ---
> diff --git a/test/sql-tap/gh-5938-wrong-string-length.c b/test/sql-tap/gh-5938-wrong-string-length.c
> new file mode 100644
> index 000000000..96823f049
> --- /dev/null
> +++ b/test/sql-tap/gh-5938-wrong-string-length.c
> @@ -0,0 +1,42 @@
> +#include <msgpuck.h>

1. We use "" for non-system headers, not <>.

> +#include "module.h"
> +
> +enum
> +{

2. Enums have { on the same line as 'enum'.

> +	BUF_SIZE = 512,
> +};
> +
> +int
> +ret_str(box_function_ctx_t *ctx, const char *args, const char *args_end)
> +{
> +	uint32_t arg_count = mp_decode_array(&args);
> +	if (arg_count != 1) {
> +		return box_error_set(__FILE__, __LINE__, ER_PROC_C, "%s",
> +			"invalid argument count");

3. You don't need "%s", you can pass the error message right away.

4. The expression is misaligned. The same for the other box_error_set().

> +	}
> +	if (mp_typeof(*args) != MP_STR) {
> +		return box_error_set(__FILE__, __LINE__, ER_PROC_C, "%s",
> +			"argument should be string");
> +	}
> +	const char* str;
> +	uint32_t str_n;
> +	str = mp_decode_str(&args, &str_n);
> +
> +	uint32_t size = mp_sizeof_array(1) + mp_sizeof_str(str_n);
> +	if (size > BUF_SIZE) {
> +		return box_error_set(__FILE__, __LINE__, ER_PROC_C, "%s",
> +			"string is too long");
> +	}