[net,2/2] net: i40e: shut up uninitialized variable warnings

Message ID 1453766332-43274-3-git-send-email-jeffrey.t.kirsher@intel.com
State New
Headers show
Series
  • Untitled series #1689
Related show

Commit Message

Jeff Kirsher Jan. 25, 2016, 11:58 p.m.
From: Arnd Bergmann <arnd@arndb.de>


intel/i40e/i40e_txrx.c: In function 'i40e_xmit_frame_ring':
intel/i40e/i40e_txrx.c:2367:20: error: 'oiph' may be used uninitialized in this function [-Werror=maybe-uninitialized]
intel/i40e/i40e_txrx.c:2317:16: note: 'oiph' was declared here
intel/i40e/i40e_txrx.c:2367:17: error: 'oudph' may be used uninitialized in this function [-Werror=maybe-uninitialized]
intel/i40e/i40e_txrx.c:2316:17: note: 'oudph' was declared here

Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Tested-by: Andrew Bowers <andrewx.bowers@intel.com>

Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>

---
 drivers/net/ethernet/intel/i40e/i40e_txrx.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

-- 
2.5.0

Comments

Vasily Averin Jan. 30, 2016, 3:33 p.m. | #1
On 26.01.2016 02:58, Jeff Kirsher wrote:
> From: Arnd Bergmann <arnd@arndb.de>

> 

> intel/i40e/i40e_txrx.c: In function 'i40e_xmit_frame_ring':

> intel/i40e/i40e_txrx.c:2367:20: error: 'oiph' may be used uninitialized in this function [-Werror=maybe-uninitialized]

> intel/i40e/i40e_txrx.c:2317:16: note: 'oiph' was declared here

> intel/i40e/i40e_txrx.c:2367:17: error: 'oudph' may be used uninitialized in this function [-Werror=maybe-uninitialized]

> intel/i40e/i40e_txrx.c:2316:17: note: 'oudph' was declared here


2364                 if ((tx_ring->flags & I40E_TXR_FLAGS_OUTER_UDP_CSUM) &&
2365                     (l4_tunnel == I40E_TXD_CTX_UDP_TUNNELING)        &&
2366                     (*cd_tunneling & I40E_TXD_CTX_QW0_EXT_IP_MASK)) {
2367                         oudph->check = ~csum_tcpudp_magic(oiph->saddr,
2368                                         oiph->daddr,
2369                                         (skb->len - skb_transport_offset(skb)),
2370                                         IPPROTO_UDP, 0);

if compiler reports that oudph and oiph can be unitialized here,
it's not enough just to set them to NULL.

Do we need probably to check that variables was initialized before access here?
i.e. add  oudph && oiph into condition?

> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

> Tested-by: Andrew Bowers <andrewx.bowers@intel.com>

> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>

> ---

>  drivers/net/ethernet/intel/i40e/i40e_txrx.c | 4 ++--

>  1 file changed, 2 insertions(+), 2 deletions(-)

> 

> diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c

> index 720516b..47bd8b3 100644

> --- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c

> +++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c

> @@ -2313,8 +2313,8 @@ static void i40e_tx_enable_csum(struct sk_buff *skb, u32 *tx_flags,

>  	struct iphdr *this_ip_hdr;

>  	u32 network_hdr_len;

>  	u8 l4_hdr = 0;

> -	struct udphdr *oudph;

> -	struct iphdr *oiph;

> +	struct udphdr *oudph = NULL;

> +	struct iphdr *oiph = NULL;

>  	u32 l4_tunnel = 0;

>  

>  	if (skb->encapsulation) {

>
Arnd Bergmann Jan. 31, 2016, 2:59 p.m. | #2
On Saturday 30 January 2016 18:33:26 Vasily Averin wrote:
> On 26.01.2016 02:58, Jeff Kirsher wrote:

> > From: Arnd Bergmann <arnd@arndb.de>

> > 

> > intel/i40e/i40e_txrx.c: In function 'i40e_xmit_frame_ring':

> > intel/i40e/i40e_txrx.c:2367:20: error: 'oiph' may be used uninitialized in this function [-Werror=maybe-uninitialized]

> > intel/i40e/i40e_txrx.c:2317:16: note: 'oiph' was declared here

> > intel/i40e/i40e_txrx.c:2367:17: error: 'oudph' may be used uninitialized in this function [-Werror=maybe-uninitialized]

> > intel/i40e/i40e_txrx.c:2316:17: note: 'oudph' was declared here

> 

> 2364                 if ((tx_ring->flags & I40E_TXR_FLAGS_OUTER_UDP_CSUM) &&

> 2365                     (l4_tunnel == I40E_TXD_CTX_UDP_TUNNELING)        &&

> 2366                     (*cd_tunneling & I40E_TXD_CTX_QW0_EXT_IP_MASK)) {

> 2367                         oudph->check = ~csum_tcpudp_magic(oiph->saddr,

> 2368                                         oiph->daddr,

> 2369                                         (skb->len - skb_transport_offset(skb)),

> 2370                                         IPPROTO_UDP, 0);

> 

> if compiler reports that oudph and oiph can be unitialized here,

> it's not enough just to set them to NULL.

> 

> Do we need probably to check that variables was initialized before access here?

> i.e. add  oudph && oiph into condition?


Sorry, I should not have mentioned my patch when it wasn't meant as
a serious submission. The patch I sent did not have a proper changelog
on it so it failed to explain it.

The reason why my patch works correctly is that the check on "l4_tunnel
== I40E_TXD_CTX_UDP_TUNNELING" means we can only get here if the
two variables have been initialized, and gcc fails to see this.

Jeff mentioned that he already had a patch for this, so I did not follow
up with a real patch.

	Arnd
Arnd Bergmann Jan. 31, 2016, 3 p.m. | #3
On Saturday 30 January 2016 19:47:36 Vasily Averin wrote:
> Patch makes safe an access to 'oiph' and 'oudph' variables

> if they was not initilized.

> 

> Signed-off-by: Vasily Averin <vvs@virtuozzo.com>

> ---

>  drivers/net/ethernet/intel/i40e/i40e_txrx.c | 3 ++-

>  1 file changed, 2 insertions(+), 1 deletion(-)

> 

> diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c

> index 47bd8b3..779f77e 100644

> --- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c

> +++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c

> @@ -2363,7 +2363,8 @@ static void i40e_tx_enable_csum(struct sk_buff *skb, u32 *tx_flags,

>                 }

>                 if ((tx_ring->flags & I40E_TXR_FLAGS_OUTER_UDP_CSUM) &&

>                     (l4_tunnel == I40E_TXD_CTX_UDP_TUNNELING)        &&

> -                   (*cd_tunneling & I40E_TXD_CTX_QW0_EXT_IP_MASK)) {

> +                   (*cd_tunneling & I40E_TXD_CTX_QW0_EXT_IP_MASK)   &&

> +                   oudph && oiph) {

>                         oudph->check = ~csum_tcpudp_magic(oiph->saddr,

>                                         oiph->daddr,

>                                         (skb->len - skb_transport_offset(skb)),


If we can actually get here with oudph==NULL or oiph==NULL, we should back
my patch instead and fix it another way.

In the version we have in net-next, that is not possible.

	Arnd

Patch

diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
index 720516b..47bd8b3 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
@@ -2313,8 +2313,8 @@  static void i40e_tx_enable_csum(struct sk_buff *skb, u32 *tx_flags,
 	struct iphdr *this_ip_hdr;
 	u32 network_hdr_len;
 	u8 l4_hdr = 0;
-	struct udphdr *oudph;
-	struct iphdr *oiph;
+	struct udphdr *oudph = NULL;
+	struct iphdr *oiph = NULL;
 	u32 l4_tunnel = 0;
 
 	if (skb->encapsulation) {