From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp3.mail.ru (smtp3.mail.ru [94.100.179.58]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id B160C469719 for ; Fri, 2 Oct 2020 15:26:22 +0300 (MSK) From: "Timur Safin" References: <578d97ed-92a0-0154-a244-f94c36f32873@tarantool.org> In-Reply-To: <578d97ed-92a0-0154-a244-f94c36f32873@tarantool.org> Date: Fri, 2 Oct 2020 15:26:21 +0300 Message-ID: <09e601d698b7$3cdafcf0$b690f6d0$@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit Content-Language: ru Subject: Re: [Tarantool-patches] [PATCH 2.X 2/7] module api: export box_key_def_dup List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: 'Vladislav Shpilevoy' , alexander.turenko@tarantool.org Cc: tarantool-patches@dev.tarantool.org : From: Vladislav Shpilevoy : Subject: Re: [Tarantool-patches] [PATCH 2.X 2/7] module api: export : box_key_def_dup : : Thanks for the patch! : : See 2 comments below. : : On 24.09.2020 19:00, Timur Safin wrote: : > Exporting `box_key_def_dup` as accessor to the internal `key_def_dup` : : 1. Do you really need this method? It looks like it can be done by : : old_parts = box_key_def_dump_parts(old_key_def); : new_key_def = box_key_def_new_ex(old_parts); : : So the method seems redundant. Meh! [I do understand that here we are on a weak grounds of pure taste preferences, which usually should be out of discussion during patchset review. But if you were asking me...] Introducing simple accessor method to already existing internal function for me, much more preferred way that introducing newer, extended variance of another, lower level function, and combining it with 2nd lower level method. Module api, ideally, should be high-level for external module developer, thus (holistically speaking) 1 higher-level wrapper is better than 2 lower-level wrappers. But, certainly, this is per personal tastes, thus we could hardly get to the final, single opinion. : : > diff --git a/src/box/key_def_api.h b/src/box/key_def_api.h : > index 8dd6eb10b..a419b712c 100644 : > --- a/src/box/key_def_api.h : > +++ b/src/box/key_def_api.h : > @@ -173,6 +173,16 @@ box_key_part_def_create(box_key_part_def_t *part); : > API_EXPORT box_key_def_t * : > box_key_def_new_ex(box_key_part_def_t *parts, uint32_t part_count); : > : > +/** : > + * Duplicate key_def. : > + * \param key_def Original key_def. : > + * : > + * \retval not NULL Duplicate of src. : > + * \retval NULL Memory error. : : 2. OCD mode intensifies. Take care! (hehe, will fix) : : :( : Lets use @ everywhere in the new code instead of \.