From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by dpdk.org (Postfix) with ESMTP id 5B779201 for ; Mon, 6 Feb 2017 06:27:56 +0100 (CET) Received: from orsmga004.jf.intel.com ([10.7.209.38]) by fmsmga104.fm.intel.com with ESMTP; 05 Feb 2017 21:27:55 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.33,340,1477983600"; d="scan'208";a="55724772" Received: from debian.sh.intel.com (HELO debian) ([10.239.67.170]) by orsmga004.jf.intel.com with ESMTP; 05 Feb 2017 21:27:54 -0800 Date: Mon, 6 Feb 2017 13:26:55 +0800 From: Tiwei Bie To: "Lu, Wenzhuo" Cc: "dev@dpdk.org" Message-ID: <20170206052655.GA2390@debian> References: <1485311962-62335-1-git-send-email-wenzhuo.lu@intel.com> <1486346972-30710-1-git-send-email-wenzhuo.lu@intel.com> <20170206023030.GA21315@debian> <6A0DE07E22DDAD4C9103DF62FEBC09093B5613B8@shsmsx102.ccr.corp.intel.com> <20170206025108.GA901@debian> <6A0DE07E22DDAD4C9103DF62FEBC09093B5613EC@shsmsx102.ccr.corp.intel.com> <20170206030821.GA10054@debian> <6A0DE07E22DDAD4C9103DF62FEBC09093B561415@shsmsx102.ccr.corp.intel.com> <20170206045706.GA17438@debian> <6A0DE07E22DDAD4C9103DF62FEBC09093B56145A@shsmsx102.ccr.corp.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <6A0DE07E22DDAD4C9103DF62FEBC09093B56145A@shsmsx102.ccr.corp.intel.com> User-Agent: Mutt/1.7.2 (2016-11-26) Subject: Re: [dpdk-dev] [PATCH v2] net/ixgbe: clean up rte_eth_dev_info_get X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 06 Feb 2017 05:27:56 -0000 On Mon, Feb 06, 2017 at 01:17:30PM +0800, Lu, Wenzhuo wrote: > > -----Original Message----- > > From: Bie, Tiwei > > Sent: Monday, February 6, 2017 12:57 PM > > To: Lu, Wenzhuo > > Cc: dev@dpdk.org > > Subject: Re: [dpdk-dev] [PATCH v2] net/ixgbe: clean up rte_eth_dev_info_get > > > > On Mon, Feb 06, 2017 at 11:45:41AM +0800, Lu, Wenzhuo wrote: > > > Hi Tiwei, > > > > > > > -----Original Message----- > > > > From: Bie, Tiwei > > > > Sent: Monday, February 6, 2017 11:08 AM > > > > To: Lu, Wenzhuo > > > > Cc: dev@dpdk.org > > > > Subject: Re: [dpdk-dev] [PATCH v2] net/ixgbe: clean up > > > > rte_eth_dev_info_get > > > > > > > > On Mon, Feb 06, 2017 at 10:59:42AM +0800, Lu, Wenzhuo wrote: > > > > > Hi Tiwei, > > > > > > > > > > > > > > > > -----Original Message----- > > > > > > From: Bie, Tiwei > > > > > > Sent: Monday, February 6, 2017 10:51 AM > > > > > > To: Lu, Wenzhuo > > > > > > Cc: dev@dpdk.org > > > > > > Subject: Re: [dpdk-dev] [PATCH v2] net/ixgbe: clean up > > > > > > rte_eth_dev_info_get > > > > > > > > > > > > On Mon, Feb 06, 2017 at 10:41:28AM +0800, Lu, Wenzhuo wrote: > > > > > > > Hi Tiwei, > > > > > > > > > > > > > > > -----Original Message----- > > > > > > > > From: Bie, Tiwei > > > > > > > > Sent: Monday, February 6, 2017 10:31 AM > > > > > > > > To: Lu, Wenzhuo > > > > > > > > Cc: dev@dpdk.org > > > > > > > > Subject: Re: [dpdk-dev] [PATCH v2] net/ixgbe: clean up > > > > > > > > rte_eth_dev_info_get > > > > > > > > > > > > > > > > On Mon, Feb 06, 2017 at 10:09:32AM +0800, Wenzhuo Lu wrote: > > > > > > > > [...] > > > > > > > > > static void ixgbe_dcb_init(struct ixgbe_hw *hw, struct > > > > > > > > > ixgbe_dcb_config *dcb_config); -static int > > > > > > > > > is_ixgbe_pmd(const char *driver_name); > > > > > > > > > +static int is_device_supported(struct rte_eth_dev *dev, > > > > > > > > > +struct eth_driver *drv); > > > > > > > > > > > > > > > > > > > > > > > > > Should be: > > > > > > > > static bool is_device_supported(struct rte_eth_dev *dev, > > > > > > > > struct eth_driver *drv); > > > > > > > O, forget to change it. Thanks. > > > > > > > > > > > > > > > > > > > > > > > > /* For Virtual Function support */ static int > > > > > > > > > eth_ixgbevf_dev_init(struct rte_eth_dev *eth_dev); @@ > > > > > > > > > -4380,16 +4380,14 @@ static int > > > > > > > > ixgbevf_dev_xstats_get_names(__rte_unused struct rte_eth_dev > > > > > > > > *dev, > > > > > > > > > ixgbe_add_rar(dev, addr, 0, 0); } > > > > > > > > > > > > > > > > > > -static int > > > > > > > > > -is_ixgbe_pmd(const char *driver_name) > > > > > > > > > +static bool > > > > > > > > > +is_device_supported(struct rte_eth_dev *dev, struct > > > > > > > > > +eth_driver > > > > > > > > > +*drv) > > > > > > > > > { > > > > > > > > > - if (!strstr(driver_name, "ixgbe")) > > > > > > > > > - return -ENOTSUP; > > > > > > > > > + if (strcmp(dev->driver->pci_drv.driver.name, > > > > > > > > > + drv->pci_drv.driver.name)) > > > > > > > > > + return FALSE; > > > > > > > > > > > > > > > > > > > > > > > > > It would be better to use `false' instead of `FALSE'. > > > > > > > I see both 'false' and 'FALSE' are defined and used. Is there > > > > > > > any reason that > > > > > > 'false' is better? > > > > > > > > > > > > > > > > > > > I think `true' and `false' are standard keywords defined and > > > > > > reserved by C. So I think it would be better to use them if the > > > > > > return type is > > > > `bool'. > > > > > O, there's no 'bool' in C. You have to define it. The same for 'false' and > > 'true'. > > > > > > > > > > > > > The `bool', `true' and `false' are all standard keywords defined and > > > > reserved by C, although the stdbool.h is not used in ixgbe. > > > > > > > > C adds this support by introducing a new header stdbool.h: > > > > > > > > #ifndef __bool_true_false_are_defined > > > > #define __bool_true_false_are_defined 1 > > > > > > > > #ifndef __cplusplus > > > > > > > > #define false 0 > > > > #define true 1 > > > > > > > > #define bool _Bool > > > > #if __STDC_VERSION__ < 199901L && __GNUC__ < 3 && > > > > !defined(__INTEL_COMPILER) > > > > typedef int _Bool; > > > > #endif > > > O, you're talking about C99. _Bool is a keyword added by it. 'bool', 'true', > > 'false' are not. That's why this header file have to define them. > > > > > > > C99 added all those as keyword, although doesn't implement all of them as > > the builtin type (e.g. int). All of them are standard keywords defined by C99. > > The `bool', `true' and `false' are defined in section 7.16 of the C99 spec [1] and > > implemented as macros: > > > > 7.16 Boolean type and values > > > > 1 The header defines four macros. > > > > 2 The macro > > bool > > expands to _Bool. > > > > 3 The remaining three macros are suitable for use in #if preprocessing > > directives. They are > > true > > which expands to the integer constant 1, > > false > > which expands to the integer constant 0, and > > __bool_true_false_are_defined > > which expands to the integer constant 1. > > > > 4 Notwithstanding the provisions of 7.1.3, a program may undefine and > > perhaps then redefine the macros bool, true, and false.222) > > > > Footnotes > > > > 222) See ''future library directions'' (7.26.7). > > > > [1] http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf > O, I see the divergence. It's about the term 'keyword'. I only count '6.4.1 Keywords'. > Anyway, as both 'false'/'true' and 'FALSE'/'TRUE' are defined. I don’t know why we cannot use any of them. If 'FALSE'/'TRUE' is not preferred, better create a new patch to clean them up. > I didn't say we cannot use FALSE/TRUE, I just suggested that false/true would be better. :-) I think the reason why introduce _Bool as builtin keyword and implement others as macros is to provide applications the ability to redefine them for compatibility issues. I think new code should follow the spec if possible. :-) Best regards, Tiwei Bie