From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from wes1-so2.wedos.net (wes1-so2.wedos.net [46.28.106.16]) by dpdk.org (Postfix) with ESMTP id CA6E02C32 for ; Wed, 1 Jun 2016 14:35:34 +0200 (CEST) Received: from pcviktorin.fit.vutbr.cz (pcviktorin.fit.vutbr.cz [147.229.13.147]) by wes1-so2.wedos.net (Postfix) with ESMTPSA id 3rKVFL3MK0z5Wv; Wed, 1 Jun 2016 14:35:34 +0200 (CEST) Date: Wed, 1 Jun 2016 14:30:18 +0200 From: Jan Viktorin To: Olivier MATZ Cc: "Hunt, David" , dev@dpdk.org, yuanhan.liu@linux.intel.com, pmatilai@redhat.com, jerin.jacob@caviumnetworks.com Message-ID: <20160601143018.5fe23534@pcviktorin.fit.vutbr.cz> In-Reply-To: <574DF6DB.4050905@6wind.com> References: <1463665501-18325-2-git-send-email-david.hunt@intel.com> <20160523143511.7d30699b@pcviktorin.fit.vutbr.cz> <574D54D6.1080409@intel.com> <20160531140652.018a03de@pcviktorin.fit.vutbr.cz> <574D95E9.4020504@intel.com> <574DF6DB.4050905@6wind.com> Organization: RehiveTech MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [dpdk-dev,v5,1/3] mempool: support external handler X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 01 Jun 2016 12:35:34 -0000 On Tue, 31 May 2016 22:40:59 +0200 Olivier MATZ wrote: > Hi, > > On 05/31/2016 03:47 PM, Hunt, David wrote: > > On 5/31/2016 1:06 PM, Jan Viktorin wrote: > >> On Tue, 31 May 2016 10:09:42 +0100 > >> "Hunt, David" wrote: > >> > >>> The *p pointer is the opaque data for a given mempool handler (ring, > >>> array, linked list, etc) > >> Again, doc comments... > >> > >> I don't like the obj_table representation to be an array of void *. I > >> could see > >> it already in DPDK for defining Ethernet driver queues, so, it's > >> probably not > >> an issue. I just say, I would prefer some basic type safety like > >> > >> struct rte_mempool_obj { > >> void *p; > >> }; > >> > >> Is there somebody with different opinions? > >> > >> [...] > > > > Comments added. I've left as a void* for the moment. > > Jan, could you please detail why you think having a > rte_mempool_obj structure brings more safety? First, void * does not say anything about the purpose of the argument. So, anybody, who is not familiar with the mempool internals would be lost for a while (as I was when studying the Ethernet queue API of DPDK). The type safety (in C, LoL) here is in the means of messing up order of arguments. When there are more void * args, you can accidently pass something wrong. DPDK has quite strict compiler settings, however, I don't consider it to be a good practice in general. When you have a named struct or a typedef, its definition usually contains doc comments describing its purposes. Nobody usually writes good doc comments for void * arguments of functions. > > For now, I'm in favor of keeping the array of void *, because > that's what we use in other mempool or ring functions. It was just a suggestion... I don't consider this to be an issue (as stated earlier). Jan > > > >>>>> +/** Structure defining a mempool handler. */ > >>>> Later in the text, I suggested to rename rte_mempool_handler to > >>>> rte_mempool_ops. > >>>> I believe that it explains the purpose of this struct better. It > >>>> would improve > >>>> consistency in function names (the *_ext_* mark is very strange and > >>>> inconsistent). > >>> I agree. I've gone through all the code and renamed to > >>> rte_mempool_handler_ops. > >> Ok. I meant rte_mempool_ops because I find the word "handler" to be > >> redundant. > > > > I prefer the use of the word handler, unless others also have opinions > > either way? > > Well, I think rte_mempool_ops is clear enough, and shorter, > so I'd vote for it. > > > Regards, > Olivier -- Jan Viktorin E-mail: Viktorin@RehiveTech.com System Architect Web: www.RehiveTech.com RehiveTech Brno, Czech Republic