From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ferruh Yigit Subject: Re: [PATCH 7/7] ethdev: use opaque user callback object Date: Mon, 4 Dec 2017 09:49:55 -0800 Message-ID: <62ea41f3-12b0-7bd0-dfdf-1374077fad8e@intel.com> References: <20171201022957.64329-1-ferruh.yigit@intel.com> <20171201022957.64329-7-ferruh.yigit@intel.com> <20171201103308.GB10384@bricha3-MOBL3.ger.corp.intel.com> <2601191342CEEE43887BDE71AB9772585FAC3718@irsmsx105.ger.corp.intel.com> <20171201131702.GA2552@bricha3-MOBL3.ger.corp.intel.com> <964acb9f-28ee-9c69-fd02-de7d3f09e429@intel.com> <20171204103123.GA9832@bricha3-MOBL3.ger.corp.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Cc: "Ananyev, Konstantin" , Thomas Monjalon , "dev@dpdk.org" , "vladz@cloudius-systems.com" To: Bruce Richardson Return-path: Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by dpdk.org (Postfix) with ESMTP id 1CB6714E8 for ; Mon, 4 Dec 2017 18:49:58 +0100 (CET) In-Reply-To: <20171204103123.GA9832@bricha3-MOBL3.ger.corp.intel.com> Content-Language: en-US List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 12/4/2017 2:31 AM, Bruce Richardson wrote: > On Fri, Dec 01, 2017 at 03:51:52PM -0800, Ferruh Yigit wrote: >> On 12/1/2017 5:17 AM, Bruce Richardson wrote: >>> On Fri, Dec 01, 2017 at 11:22:12AM +0000, Ananyev, Konstantin wrote: >>>> >>>> >>>>> -----Original Message----- >>>>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Bruce Richardson >>>>> Sent: Friday, December 1, 2017 10:33 AM >>>>> To: Yigit, Ferruh >>>>> Cc: Thomas Monjalon ; dev@dpdk.org; vladz@cloudius-systems.com >>>>> Subject: Re: [dpdk-dev] [PATCH 7/7] ethdev: use opaque user callback object >>>>> >>>>> On Fri, Dec 01, 2017 at 02:29:57AM +0000, Ferruh Yigit wrote: >>>>>> "struct rte_eth_rxtx_callback" is defined as internal data structure but >>>>>> used in public APIs. >>>>>> >>>>>> Checking the API documentation shows that intention was using this >>>>>> object as opaque object. Data structure only used in delete APIs which >>>>>> doesn't require to know the internals of the data structure. >>>>>> >>>>>> Converting callback parameter in API to void pointer should not require >>>>>> any modification in user application because this data structure was >>>>>> already marked as internal and only should be used as pointer in >>>>>> application. >>>>>> >>>>>> Signed-off-by: Ferruh Yigit >>>>>> --- >>>>> >>>>> I disagree on this patch. The structure itself is not exposed, only the >>>>> name, since it is only passed around as a pointer, so there is no need >>>>> to change the parameters to void pointer. It's a named opaque type. >>>> >>>> Personally I think it would be better to do visa-versa: >>>> make rte_eth_add_(rx|tx)_callback() to return struct rte_eth_rxtx_callback * >>>> instead of void *. >>>> Konstantin >>>> >>> I didn't realise that it did, so definite +1 to that suggestion. >> >> No issue on having a named opaque type, but unfortunately struct is exposed >> because of inline functions again. >> It has been moved into rte_ethdev_core.h but accessible by applications. >> >> And since intention is an opaque type, because of "void *" return types, I >> thought it is better to hide type completely so that application can't access >> details. > > I wouldn't be worried about applications being able to get into the > structure. The only compelling reason for me to make the type opaque > would be for ABI compatibility, and since that is not a factor here, I > don't see the point in changing it to a void *. OK, I will update as suggested. > > /Bruce >