Tarantool development patches archive
 help / color / mirror / Atom feed
From: Vladislav Shpilevoy via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: imeevma@tarantool.org
Cc: tarantool-patches@dev.tarantool.org
Subject: Re: [Tarantool-patches] [PATCH v1 1/2] sql: disallow explicit cast of BOOLEAN to number
Date: Mon, 26 Jul 2021 22:11:42 +0200	[thread overview]
Message-ID: <5e432ccf-58ad-60cf-9506-2574c280539b@tarantool.org> (raw)
In-Reply-To: <894e6f0fb325f06b802b462615f7d1cd1b2c0a5f.1626880058.git.imeevma@gmail.com>

Thanks for the patch!

See 2 comments below.

> diff --git a/src/box/sql/mem.c b/src/box/sql/mem.c
> index 6b95e41d3..5c44bfdfc 100644
> --- a/src/box/sql/mem.c
> +++ b/src/box/sql/mem.c
> @@ -683,24 +674,23 @@ str_to_bool(struct Mem *mem)
>  {
>  	assert(mem->type == MEM_TYPE_STR);
>  	char *str = mem->z;
> +	uint32_t len = mem->n;
>  	bool b;
>  	const char *str_true = "TRUE";
>  	const char *str_false = "FALSE";
>  	uint32_t len_true = strlen(str_true);
>  	uint32_t len_false = strlen(str_false);
>  
> -	for (; str[0] == ' '; str++);
> -	if (strncasecmp(str, str_true, len_true) == 0) {
> +	for (; isspace(str[0]); str++, len--);
> +	for (; isspace(str[len - 1]); len--);
> +	if (len != len_true && len != len_false)
> +		return -1;
> +
> +	if (len == len_true && strncasecmp(str, str_true, len) == 0)
>  		b = true;
> -		str += len_true;
> -	} else if (strncasecmp(str, str_false, len_false) == 0) {
> +	else if (len == len_false && strncasecmp(str, str_false, len) == 0)
>  		b = false;
> -		str += len_false;
> -	} else {
> -		return -1;
> -	}
> -	for (; str[0] == ' '; str++);
> -	if (str[0] != '\0')
> +	else
>  		return -1;

1. Why did you change str_to_bool() if the patch is only about
numbers <-> bool?

>  	mem_set_bool(mem, b);
>  	return 0;
> @@ -1074,19 +1036,11 @@ mem_cast_explicit(struct Mem *mem, enum field_type type)
>  	case FIELD_TYPE_INTEGER:
>  		return mem_to_int(mem);
>  	case FIELD_TYPE_BOOLEAN:
> -		switch (mem->type) {
> -		case MEM_TYPE_BOOL:
> +		if (mem->type == MEM_TYPE_BOOL)
>  			return 0;
> -		case MEM_TYPE_INT:
> -		case MEM_TYPE_UINT:
> -			return int_to_bool(mem);
> -		case MEM_TYPE_STR:
> +		if (mem->type == MEM_TYPE_STR)
>  			return str_to_bool(mem);
> -		case MEM_TYPE_DOUBLE:
> -			return double_to_bool(mem);
> -		default:
> -			return -1;

2. I would propose to keep the switch-case. Otherwise you are
going to jump back and forth between if and switch when these
places will be changed again.

  reply	other threads:[~2021-07-26 20:11 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-21 15:10 [Tarantool-patches] [PATCH v1 0/2] Fix explicit casts Mergen Imeev via Tarantool-patches
2021-07-21 15:10 ` [Tarantool-patches] [PATCH v1 1/2] sql: disallow explicit cast of BOOLEAN to number Mergen Imeev via Tarantool-patches
2021-07-26 20:11   ` Vladislav Shpilevoy via Tarantool-patches [this message]
2021-07-21 15:10 ` [Tarantool-patches] [PATCH v1 2/2] sql: disallow explicit cast of VARBINARY " Mergen Imeev via Tarantool-patches
2021-07-26 20:12   ` Vladislav Shpilevoy via Tarantool-patches

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5e432ccf-58ad-60cf-9506-2574c280539b@tarantool.org \
    --to=tarantool-patches@dev.tarantool.org \
    --cc=imeevma@tarantool.org \
    --cc=v.shpilevoy@tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH v1 1/2] sql: disallow explicit cast of BOOLEAN to number' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox