[tarantool-patches] Re: [PATCH v7 2/7] box: introduce OPT_ARRAY opt_type to decode arrays
Konstantin Osipov
kostja at tarantool.org
Wed May 23 20:53:45 MSK 2018
* Kirill Shcherbatov <kshcherbatov at tarantool.org> [18/05/23 20:33]:
> 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;
> + break;
> default:
> unreachable();
> }
> diff --git a/src/box/opt_def.h b/src/box/opt_def.h
> index 633832a..71fef3a 100644
> --- a/src/box/opt_def.h
> +++ b/src/box/opt_def.h
> @@ -47,12 +47,14 @@ enum opt_type {
> OPT_STR, /* char[] */
> OPT_STRPTR, /* char* */
> OPT_ENUM, /* enum */
> + OPT_ARRAY, /* array */
> opt_type_MAX,
> };
>
> extern const char *opt_type_strs[];
>
> typedef int64_t (*opt_def_to_enum_cb)(const char *str, uint32_t len);
> +typedef int (*opt_def_to_array_cb)(const char **str, uint32_t len, char *opt);
While the previous callback is self-explanatory, this one requires
a comment. Actually a comment for both won't hurt. What should the
callback return? who does own the memory used by the return value?
What is the error handling convention?
> struct opt_def {
> const char *name;
> @@ -64,19 +66,26 @@ struct opt_def {
> int enum_size;
> const char **enum_strs;
> uint32_t enum_max;
> - /** If not NULL, used to get a enum value by a string. */
> - opt_def_to_enum_cb to_enum;
> + /** If not NULL, used to get a value by a string. */
The comment became worse, not better.
> + union {
> + opt_def_to_enum_cb to_enum;
> + opt_def_to_array_cb to_array;
> + };
> };
Otherwise LGTM.
--
Konstantin Osipov, Moscow, Russia, +7 903 626 22 32
http://tarantool.io - www.twitter.com/kostja_osipov
More information about the Tarantool-patches
mailing list