Message ID | 1456426121-21423-9-git-send-email-aconole@redhat.com (mailing list archive) |
---|---|
State | Superseded, archived |
Delegated to: | Thomas Monjalon |
Headers |
Return-Path: <dev-bounces@dpdk.org> X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [IPv6:::1]) by dpdk.org (Postfix) with ESMTP id 2F5CC2C0E; Thu, 25 Feb 2016 19:48:57 +0100 (CET) Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by dpdk.org (Postfix) with ESMTP id 1F7C52BDB for <dev@dpdk.org>; Thu, 25 Feb 2016 19:48:49 +0100 (CET) Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (Postfix) with ESMTPS id 9D9BF85545; Thu, 25 Feb 2016 18:48:48 +0000 (UTC) Received: from aconole-fed23.bos.redhat.com (dhcp-25-213.bos.redhat.com [10.18.25.213]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u1PImfwq000303; Thu, 25 Feb 2016 13:48:48 -0500 From: Aaron Conole <aconole@redhat.com> To: dev@dpdk.org Date: Thu, 25 Feb 2016 13:48:41 -0500 Message-Id: <1456426121-21423-9-git-send-email-aconole@redhat.com> In-Reply-To: <1456426121-21423-1-git-send-email-aconole@redhat.com> References: <1456426121-21423-1-git-send-email-aconole@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.22 Subject: [dpdk-dev] [PATCH 8/8] drivers/net/ixgbe: Fix uninitialized warning X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK <dev.dpdk.org> List-Unsubscribe: <http://dpdk.org/ml/options/dev>, <mailto:dev-request@dpdk.org?subject=unsubscribe> List-Archive: <http://dpdk.org/ml/archives/dev/> List-Post: <mailto:dev@dpdk.org> List-Help: <mailto:dev-request@dpdk.org?subject=help> List-Subscribe: <http://dpdk.org/ml/listinfo/dev>, <mailto:dev-request@dpdk.org?subject=subscribe> Errors-To: dev-bounces@dpdk.org Sender: "dev" <dev-bounces@dpdk.org> |
Commit Message
Aaron Conole
Feb. 25, 2016, 6:48 p.m. UTC
Silence a compiler warning that this variable may be used uninitialized.
Signed-off-by: Aaron Conole <aconole@redhat.com>
---
drivers/net/ixgbe/ixgbe_rxtx.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
Comments
On 02/25/2016 08:48 PM, Aaron Conole wrote: > Silence a compiler warning that this variable may be used uninitialized. > > Signed-off-by: Aaron Conole <aconole@redhat.com> > --- > drivers/net/ixgbe/ixgbe_rxtx.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c > index e95e6b7..775edc7 100644 > --- a/drivers/net/ixgbe/ixgbe_rxtx.c > +++ b/drivers/net/ixgbe/ixgbe_rxtx.c > @@ -1563,7 +1563,7 @@ ixgbe_recv_pkts_lro(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts, > struct ixgbe_rx_entry *rxe; > struct ixgbe_scattered_rx_entry *sc_entry; > struct ixgbe_scattered_rx_entry *next_sc_entry; > - struct ixgbe_rx_entry *next_rxe; > + struct ixgbe_rx_entry *next_rxe = NULL; > struct rte_mbuf *first_seg; > struct rte_mbuf *rxm; > struct rte_mbuf *nmb; > @@ -1740,7 +1740,7 @@ next_desc: > * the pointer to the first mbuf at the NEXTP entry in the > * sw_sc_ring and continue to parse the RX ring. > */ > - if (!eop) { > + if (!eop && next_rxe) { > rxm->next = next_rxe->mbuf; > next_sc_entry->fbuf = first_seg; > goto next_desc; > The patch looks ok as such, but then again warning looks like a false positive to me: assignment and dereferencing depend on the same value of eop, which cannot change between the two. CC'ing the maintainers for attention... - Panu -
On 10/03/2016 13:42, Panu Matilainen wrote: > On 02/25/2016 08:48 PM, Aaron Conole wrote: >> Silence a compiler warning that this variable may be used uninitialized. >> >> Signed-off-by: Aaron Conole <aconole@redhat.com> [..] > > The patch looks ok as such, but then again warning looks like a false > positive to me: assignment and dereferencing depend on the same value of > eop, which cannot change between the two. In two minds about this. It is a logical impossibility, but these days optimising compilers are getting very aggressive. For instance GCC has a delightfully-named -fdelete-null-pointer-checks option, which caused security holes.. ..Remy
On 03/10/2016 04:45 PM, Remy Horton wrote: > > On 10/03/2016 13:42, Panu Matilainen wrote: >> On 02/25/2016 08:48 PM, Aaron Conole wrote: >>> Silence a compiler warning that this variable may be used uninitialized. >>> >>> Signed-off-by: Aaron Conole <aconole@redhat.com> > [..] >> >> The patch looks ok as such, but then again warning looks like a false >> positive to me: assignment and dereferencing depend on the same value of >> eop, which cannot change between the two. > > In two minds about this. It is a logical impossibility, but these days > optimising compilers are getting very aggressive. For instance GCC has a > delightfully-named -fdelete-null-pointer-checks option, which caused > security holes.. Indeed, that's why silencing a false positive (assuming it actually is one) by throwing some more NULL-checks for the allegedly impossible makes me a bit nervous. Besides compiler optimizations going crazy, I've seen such extra NULL-checks turn into actual bugs when surroundings subtly change. - Panu -
On 10/03/2016 15:03, Panu Matilainen wrote: > On 03/10/2016 04:45 PM, Remy Horton wrote: [...] >> In two minds about this. It is a logical impossibility, but these days >> optimising compilers are getting very aggressive. For instance GCC has a >> delightfully-named -fdelete-null-pointer-checks option, which caused >> security holes.. > > Indeed, that's why silencing a false positive (assuming it actually is > one) by throwing some more NULL-checks for the allegedly impossible > makes me a bit nervous. Besides compiler optimizations going crazy, I've > seen such extra NULL-checks turn into actual bugs when surroundings > subtly change. It cuts both ways. To anyone who is not an active compiler engineer, fixing a warning being /more/ likley to screw things up is quite a big thing. Do we want to turn off warnings or turn off optimisations.. :) ..Remy
> -----Original Message----- > From: Aaron Conole [mailto:aconole@redhat.com] > Sent: Friday, February 26, 2016 2:49 AM > To: dev@dpdk.org > Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Zhang, Helin > <helin.zhang@intel.com>; Ananyev, Konstantin > <konstantin.ananyev@intel.com>; Richardson, Bruce > <bruce.richardson@intel.com> > Subject: [PATCH 8/8] drivers/net/ixgbe: Fix uninitialized warning > > Silence a compiler warning that this variable may be used uninitialized. > > Signed-off-by: Aaron Conole <aconole@redhat.com> Acked-by: Helin Zhang <helin.zhang@intel.com> > --- > drivers/net/ixgbe/ixgbe_rxtx.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c > index e95e6b7..775edc7 100644 > --- a/drivers/net/ixgbe/ixgbe_rxtx.c > +++ b/drivers/net/ixgbe/ixgbe_rxtx.c > @@ -1563,7 +1563,7 @@ ixgbe_recv_pkts_lro(void *rx_queue, struct rte_mbuf > **rx_pkts, uint16_t nb_pkts, > struct ixgbe_rx_entry *rxe; > struct ixgbe_scattered_rx_entry *sc_entry; > struct ixgbe_scattered_rx_entry *next_sc_entry; > - struct ixgbe_rx_entry *next_rxe; > + struct ixgbe_rx_entry *next_rxe = NULL; > struct rte_mbuf *first_seg; > struct rte_mbuf *rxm; > struct rte_mbuf *nmb; > @@ -1740,7 +1740,7 @@ next_desc: > * the pointer to the first mbuf at the NEXTP entry in the > * sw_sc_ring and continue to parse the RX ring. > */ > - if (!eop) { > + if (!eop && next_rxe) { > rxm->next = next_rxe->mbuf; > next_sc_entry->fbuf = first_seg; > goto next_desc; > -- > 2.5.0
diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c index e95e6b7..775edc7 100644 --- a/drivers/net/ixgbe/ixgbe_rxtx.c +++ b/drivers/net/ixgbe/ixgbe_rxtx.c @@ -1563,7 +1563,7 @@ ixgbe_recv_pkts_lro(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts, struct ixgbe_rx_entry *rxe; struct ixgbe_scattered_rx_entry *sc_entry; struct ixgbe_scattered_rx_entry *next_sc_entry; - struct ixgbe_rx_entry *next_rxe; + struct ixgbe_rx_entry *next_rxe = NULL; struct rte_mbuf *first_seg; struct rte_mbuf *rxm; struct rte_mbuf *nmb; @@ -1740,7 +1740,7 @@ next_desc: * the pointer to the first mbuf at the NEXTP entry in the * sw_sc_ring and continue to parse the RX ring. */ - if (!eop) { + if (!eop && next_rxe) { rxm->next = next_rxe->mbuf; next_sc_entry->fbuf = first_seg; goto next_desc;