From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id F1BBF2479E for ; Wed, 23 May 2018 13:53:47 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id U1-4PrVVYFMH for ; Wed, 23 May 2018 13:53:47 -0400 (EDT) Received: from smtp48.i.mail.ru (smtp48.i.mail.ru [94.100.177.108]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id B133824793 for ; Wed, 23 May 2018 13:53:47 -0400 (EDT) Date: Wed, 23 May 2018 20:53:45 +0300 From: Konstantin Osipov Subject: [tarantool-patches] Re: [PATCH v7 2/7] box: introduce OPT_ARRAY opt_type to decode arrays Message-ID: <20180523175345.GC4266@atlas> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-subscribe: List-owner: List-post: List-archive: To: tarantool-patches@freelists.org Cc: v.shpilevoy@tarantool.org, Kirill Shcherbatov * Kirill Shcherbatov [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