[dpdk-dev,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>
---
 drivers/net/ixgbe/ixgbe_rxtx.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
  

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