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 A9F2824E91 for ; Thu, 31 May 2018 13:37:01 -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 0TLhUjB2Ebnm for ; Thu, 31 May 2018 13:37:01 -0400 (EDT) Received: from smtp33.i.mail.ru (smtp33.i.mail.ru [94.100.177.93]) (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 ABD6E24FB8 for ; Thu, 31 May 2018 13:36:54 -0400 (EDT) Subject: [tarantool-patches] Re: [PATCH v1 3/4] box: introduce box_space_id_by_name References: <95c32aa69c7040aa5a63164e19de8ac8f06d9ce3.1527765756.git.kshcherbatov@tarantool.org> From: Vladislav Shpilevoy Message-ID: <91765db9-575c-9d10-c678-ff1f3b230735@tarantool.org> Date: Thu, 31 May 2018 20:36:52 +0300 MIME-Version: 1.0 In-Reply-To: <95c32aa69c7040aa5a63164e19de8ac8f06d9ce3.1527765756.git.kshcherbatov@tarantool.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit 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, Kirill Shcherbatov Thanks for the patch! See 5 comments below. On 31/05/2018 14:22, Kirill Shcherbatov wrote: > Part of #3273. > --- > src/box/box.cc | 10 ++++++++-- > src/box/box.h | 14 ++++++++++++++ > 2 files changed, 22 insertions(+), 2 deletions(-) > > diff --git a/src/box/box.cc b/src/box/box.cc > index 02722d6..9540d85 100644 > --- a/src/box/box.cc > +++ b/src/box/box.cc > @@ -877,7 +877,7 @@ box_return_tuple(box_function_ctx_t *ctx, box_tuple_t *tuple) > > /* schema_find_id()-like method using only public API */ > uint32_t > -box_space_id_by_name(const char *name, uint32_t len) > +space_id_by_name(uint32_t system_space_id, const char *name, uint32_t len) 1. The comment now is wrong - space_id_by_name will be used exactly for non-public API. 2. space_id_by_name must not take system_space_id as an argument. It must always use BOX_SPACE_ID. Do this: /** * ... * * Return 0 on success, -1 on error. * @param[out] space_id BOX_ID_NIL - not found. Else space_id is * saved. * * ... */ static inline int space_id_by_name_impl(system_space_id, name, len, uint32_t *space_id); uint32_t space_id_by_name(name, len) { return space_id_by_name_impl(BOX_SPACE_ID, name, len); } uint32_t box_space_id_by_name(name, len) { return space_id_by_name_impl(BOX_VSPACE_ID, name, len); } > { > if (len > BOX_NAME_MAX) > return BOX_ID_NIL; > diff --git a/src/box/box.h b/src/box/box.h > index bdd5d5c..ba33113 100644 > --- a/src/box/box.h > +++ b/src/box/box.h > @@ -220,6 +220,20 @@ API_EXPORT int > box_return_tuple(box_function_ctx_t *ctx, box_tuple_t *tuple); > > /** > + * Find space id by name in specified system space. > + * > + * This function performs SELECT request to _vspace system space. 3. Not _vspace. > + * \param system_space_id space to lookup name. > + * \param name space name > + * \param len length of \a name > + * \retval BOX_ID_NIL on error or if not found (check box_error_last()) > + * \retval space_id otherwise > + * \sa box_index_id_by_name > + */ > +uint32_t > +space_id_by_name(uint32_t system_space_id, const char *name, uint32_t len); 4. space_id_by_name is not public function. You must not put it inside /** \cond public */. It must be in schema.h like other functions. 5. Why can not you just port schema_find_id to C? > + > +/** > * Find space id by name. > * > * This function performs SELECT request to _vspace system space. >