[tarantool-patches] Re: [PATCH v7 2/7] box: introduce OPT_ARRAY opt_type to decode arrays

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

Thanks for the patch! See 1 comment below.

On 23/05/2018 17:05, Kirill Shcherbatov wrote:
> As we need to store Checks array in opt_def MsgPack in future
> introduced special type and decode callback to_array used in opt_set
> function.
> 
> Part of #3272.
> ---
>   src/box/opt_def.c |  9 +++++++++
>   src/box/opt_def.h | 19 ++++++++++++++-----
>   2 files changed, 23 insertions(+), 5 deletions(-)
> 
> diff --git a/src/box/opt_def.c b/src/box/opt_def.c
> index cd93c23..c8440c9 100644
> --- a/src/box/opt_def.c
> +++ b/src/box/opt_def.c
> @@ -44,6 +44,7 @@ const char *opt_type_strs[] = {
>   	/* [OPT_STR]	= */ "string",
>   	/* [OPT_STRPTR] = */ "string",
>   	/* [OPT_ENUM]   = */ "enum",
> +	/* [OPT_ARRAY]  = */ "array",
>   };
>   
>   static int
> @@ -135,6 +136,14 @@ opt_set(void *opts, const struct opt_def *def, const char **val,
>   			unreachable();
>   		};
>   		break;
> +	case OPT_ARRAY:
> +		if (mp_typeof(**val) != MP_ARRAY)
> +			return -1;
> +		ival = mp_decode_array(val);
> +		assert(def->to_array != NULL);
> +		if (def->to_array(val, ival, opt) != 0)
> +			return -1;

1. If to_array is failed, the error is reset in opts_parse_key:

snprintf(errmsg, sizeof(errmsg), "'%.*s' must be %s",
	 key_len, key, opt_type_strs[def->type]);
diag_set(ClientError, errcode, field_no, errmsg);

So the original error is lost. You should refactor opt_set
so that it sets diag on type mismatch instead of doing it in
opts_parse_key. And opts_parse_key does nothing on error in opt_set -
just return -1.

> +		break;
>   	default:
>   		unreachable();
>   	}