[Tarantool-patches] [PATCH v1 01/21] sql: refactor CHAR() function
Vladislav Shpilevoy
v.shpilevoy at tarantool.org
Sat Oct 30 02:42:42 MSK 2021
Hi! Thanks for the fixes!
See 3 comments below.
> diff --git a/src/box/sql/func.c b/src/box/sql/func.c
> index afe34f7f0..dee28b852 100644
> --- a/src/box/sql/func.c
> +++ b/src/box/sql/func.c
> @@ -717,6 +717,57 @@ func_substr_characters(struct sql_context *ctx, int argc, struct Mem *argv)
> ctx->is_aborted = true;
> }
>
> +/**
> + * Implementation of the CHAR() function.
> + *
> + * This function takes zero or more arguments, each of which is an integer. It
> + * constructs a string where each character of the string is the unicode
> + * character for the corresponding integer argument.
> + *
> + * If an argument is negative or greater than 0x10ffff, the symbol "�" is used.
> + * Symbol '\0' used instead of NULL argument.
> + */
> +static void
> +func_char(struct sql_context *ctx, int argc, struct Mem *argv)
> +{
> + if (argc == 0)
> + return mem_set_str_static(ctx->pOut, "", 0);
> + struct region *region = &fiber()->gc;
> + size_t svp = region_used(region);
> + UChar32 *buf = region_alloc(region, argc * sizeof(*buf));
1. Would be better to use region_alloc_array(). Otherwise you
risk to get misaligned data.
> + if (buf == NULL) {
> + ctx->is_aborted = true;
2. Need to use diag_set() here.
> + return;
> + }
> + int len = 0;
> + for (int i = 0; i < argc; ++i) {
> + if (mem_is_null(&argv[i]))
> + buf[i] = 0;
> + else if (!mem_is_uint(&argv[i]) || argv[i].u.u > 0x10ffff)
> + buf[i] = 0xfffd;
> + else
> + buf[i] = argv[i].u.u;
> + len += U8_LENGTH(buf[i]);
> + }
> +
> + char *str = sqlDbMallocRawNN(sql_get(), len);
> + if (str == NULL) {
> + ctx->is_aborted = true;
3. region leaks here, doesn't it?
More information about the Tarantool-patches
mailing list