From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by dpdk.org (Postfix) with ESMTP id EE51B5689 for ; Mon, 24 Sep 2018 17:06:38 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by orsmga103.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 24 Sep 2018 08:06:37 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.54,298,1534834800"; d="scan'208";a="91454064" Received: from fyigit-mobl.ger.corp.intel.com (HELO [10.237.221.39]) ([10.237.221.39]) by fmsmga004.fm.intel.com with ESMTP; 24 Sep 2018 08:06:35 -0700 To: Thomas Monjalon , Rahul Lakkireddy , Wenzhuo Lu , Qi Zhang , Xiao Wang , Konstantin Ananyev , Rasesh Mody , Harish Patil , Shahed Shaikh , Yong Wang Cc: dev@dpdk.org References: <20180920001853.23454-1-thomas@monjalon.net> From: Ferruh Yigit Openpgp: preference=signencrypt Message-ID: Date: Mon, 24 Sep 2018 16:06:34 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <20180920001853.23454-1-thomas@monjalon.net> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH] drivers/net: do not redefine bool 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, 24 Sep 2018 15:06:39 -0000 On 9/20/2018 1:18 AM, Thomas Monjalon wrote: > When trying to include stdbool.h in DPDK base headers, there are a lot > of conflicts with drivers which redefine bool/true/false > in their compatibility layer. > > It is fixed by including stdbool.h in these drivers. > Some errors with usage of bool type are also fixed in some drivers. > > Note: the driver qede has a surprising mix of bool and int: > (~p_iov->b_pre_fp_hsi & ETH_HSI_VER_MINOR) > where the first variable is boolean and the version is a number. > It is replaced by > !p_iov->b_pre_fp_hsi > > Signed-off-by: Thomas Monjalon <...> > diff --git a/drivers/net/e1000/base/e1000_osdep.h b/drivers/net/e1000/base/e1000_osdep.h > index b8868049f..556ed1742 100644 > --- a/drivers/net/e1000/base/e1000_osdep.h > +++ b/drivers/net/e1000/base/e1000_osdep.h > @@ -35,6 +35,7 @@ > #ifndef _E1000_OSDEP_H_ > #define _E1000_OSDEP_H_ > > +#include > #include > #include > #include > @@ -87,7 +88,6 @@ typedef int64_t s64; > typedef int32_t s32; > typedef int16_t s16; > typedef int8_t s8; > -typedef int bool; > > #define __le16 u16 > #define __le32 u32 > @@ -192,7 +192,4 @@ static inline uint16_t e1000_read_addr16(volatile void *addr) > #define ETH_ADDR_LEN 6 > #endif > > -#define false FALSE > -#define true TRUE TRUE and FALSE also defined in this patch, can we remove them too? > - > #endif /* _E1000_OSDEP_H_ */ > diff --git a/drivers/net/fm10k/base/fm10k_osdep.h b/drivers/net/fm10k/base/fm10k_osdep.h > index 199ebd8ea..9665239fd 100644 > --- a/drivers/net/fm10k/base/fm10k_osdep.h > +++ b/drivers/net/fm10k/base/fm10k_osdep.h > @@ -34,6 +34,7 @@ POSSIBILITY OF SUCH DAMAGE. > #ifndef _FM10K_OSDEP_H_ > #define _FM10K_OSDEP_H_ > > +#include > #include > #include > #include > @@ -61,12 +62,6 @@ POSSIBILITY OF SUCH DAMAGE. > > #define FALSE 0 > #define TRUE 1 > -#ifndef false > -#define false FALSE > -#endif > -#ifndef true > -#define true TRUE > -#endif Same here, TRUE and FALSE defined in this header and used in .c files one or two places, what about remove them and convert usage to "true" and "false" <...> > diff --git a/drivers/net/ixgbe/base/ixgbe_osdep.h b/drivers/net/ixgbe/base/ixgbe_osdep.h > index bb5dfd2af..39e9118aa 100644 > --- a/drivers/net/ixgbe/base/ixgbe_osdep.h > +++ b/drivers/net/ixgbe/base/ixgbe_osdep.h > @@ -36,6 +36,7 @@ > #define _IXGBE_OS_H_ > > #include > +#include > #include > #include > #include > @@ -70,8 +71,6 @@ > #define FALSE 0 > #define TRUE 1 Same again, can we remove TRUE and FALSE <...> > diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c > index cee886754..c272a4112 100644 > --- a/drivers/net/ixgbe/ixgbe_ethdev.c > +++ b/drivers/net/ixgbe/ixgbe_ethdev.c > @@ -2527,7 +2527,9 @@ ixgbe_dev_start(struct rte_eth_dev *dev) > struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev); > struct rte_intr_handle *intr_handle = &pci_dev->intr_handle; > uint32_t intr_vector = 0; > - int err, link_up = 0, negotiate = 0; > + int err; > + bool negotiate = false; > + bool link_up = false; "link_up" is used in assignment to a single bit in uint16_t: dev->data->dev_link.link_status = link_up; When "link_up" is bool, should we change that line to: if (link_up) dev->data->dev_link.link_status = 1; else dev->data->dev_link.link_status = 0; <...> > @@ -3870,7 +3872,7 @@ ixgbevf_dev_info_get(struct rte_eth_dev *dev, > > static int > ixgbevf_check_link(struct ixgbe_hw *hw, ixgbe_link_speed *speed, > - int *link_up, int wait_to_complete) > + bool *link_up, int wait_to_complete) Also need to change "wait_to_complete" to bool because below changes start sending bool type to this function. <...> > diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c > index ae21f04a1..2dc14c47f 100644 > --- a/drivers/net/ixgbe/ixgbe_rxtx.c > +++ b/drivers/net/ixgbe/ixgbe_rxtx.c > @@ -2025,7 +2025,7 @@ ixgbe_recv_pkts_lro(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts, > struct ixgbe_rx_entry *next_rxe = NULL; > struct rte_mbuf *first_seg; > struct rte_mbuf *rxm; > - struct rte_mbuf *nmb; > + struct rte_mbuf *nmb = NULL; This change is unrelated. Can we separate this one?