From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpng1.m.smailru.net (smtpng1.m.smailru.net [94.100.181.251]) (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 A88194696C3 for ; Mon, 27 Apr 2020 12:19:22 +0300 (MSK) Date: Mon, 27 Apr 2020 12:12:13 +0300 From: Igor Munkin Message-ID: <20200427091213.GL11314@tarantool.org> References: <4177aec25c7ff283575a0ccb3a3a62d2ee51fde8.1587600640.git.v.shpilevoy@tarantool.org> <20200425002109.GK11314@tarantool.org> <3a8fe601-d017-bc12-cf62-974f0177ccd1@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <3a8fe601-d017-bc12-cf62-974f0177ccd1@tarantool.org> Subject: Re: [Tarantool-patches] [PATCH v2 3/3] box: replace port_tuple with port_c everywhere List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Vladislav Shpilevoy Cc: tarantool-patches@dev.tarantool.org Vlad, Thanks for the fixes. On 26.04.20, Vladislav Shpilevoy wrote: > Hi! Thanks for the review! > > On 25/04/2020 02:21, Igor Munkin wrote: > > Vlad, > > > > Thanks for the patch! Please consider the comments I left below. > > > > On 23.04.20, Vladislav Shpilevoy wrote: > >> diff --git a/src/box/port.c b/src/box/port.c > >> index 2c1fadb5c..9d9fc1dbc 100644 > >> --- a/src/box/port.c > >> +++ b/src/box/port.c > >> @@ -38,106 +38,15 @@ > >> + * result data when it fits into an object from the pool. > >> */ > >> static struct mempool port_entry_pool; > >> > >> enum { > >> - PORT_ENTRY_SIZE = MAX(sizeof(struct port_c_entry), > >> - sizeof(struct port_tuple_entry)), > >> + PORT_ENTRY_SIZE = sizeof(struct port_c_entry), > > > > Minor: PORT_ENTRY_SIZE is introduced in the first patch of the series. > > After applying these changes it looks excess. Feel free to ignore if you > > see any rationale to leave this constant. > > If I change it, I need to change its usage place too, and increase > the diff. If we want to make the diff even smaller and remove this > constant, it would be better to merge this commit and the first one. > I can do that, if you want. I see just little occurences and all: | $ grep -rnF 'PORT_ENTRY_SIZE' src | cut -f 1 -d ':' | sort | uniq -c | 5 src/box/port.c But, this comment definitely doesn't deserve merging indepentent commits. Let's leave this as is or until Nikita asks to fix it in another way. > -- Best regards, IM