From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ferruh Yigit Subject: Re: [PATCH] drivers/net: do not redefine bool Date: Tue, 25 Sep 2018 09:03:27 +0100 Message-ID: <840d806c-d75c-f990-5883-1315ea37f95b@intel.com> References: <20180920001853.23454-1-thomas@monjalon.net> <1920676.XclAJlGjTg@xps> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Cc: Rahul Lakkireddy , Wenzhuo Lu , Qi Zhang , Xiao Wang , Konstantin Ananyev , Rasesh Mody , Harish Patil , Shahed Shaikh , Yong Wang , dev@dpdk.org To: Thomas Monjalon Return-path: Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by dpdk.org (Postfix) with ESMTP id DCAFF2B8C for ; Tue, 25 Sep 2018 10:03:36 +0200 (CEST) In-Reply-To: <1920676.XclAJlGjTg@xps> 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 9/24/2018 5:59 PM, Thomas Monjalon wrote: > 24/09/2018 17:06, Ferruh Yigit: >> On 9/20/2018 1:18 AM, Thomas Monjalon wrote: >>> -#define false FALSE >>> -#define true TRUE >> >> TRUE and FALSE also defined in this patch, can we remove them too? > > I don't see the need to remove TRUE and FALSE. > The base drivers use them on other platforms, and it is convenient to not > change the base drivers. Not needed, but previously it was only TRUE & FALSE, and true & false was define to them. Now there are TRUE & FALSE from header files and true & false from stdbool and these pairs used interchangeably, I thought it can better to unify the usage to stdbool ones. > > [...] >>> 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. > > [...] >>> --- 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? > > Yes it looks unrelated but it becomes necessary when including stdbool.h. > I don't know the root cause, but yes, it may deserve a separate commit. > Maybe an ixgbe maintainer can take care of it? Why becomes necessary? Does it give a build warning etc? My concern is this is in data path, one extra assignment, it would be better to confirm it is really needed.