Re: [PATCH v2 1/2] net/avf: fix unused variables and label

From: Bruce Richardson <bruce.richardson@intel.com>
To: Ferruh Yigit <ferruh.yigit@intel.com>
Cc: Jingjing Wu <jingjing.wu@intel.com>,
	Wenzhuo Lu <wenzhuo.lu@intel.com>,
	dev@dpdk.org, bluca@debian.org, Qi Zhang <qi.z.zhang@intel.com>,
	stable@dpdk.org
Subject: Re: [PATCH v2 1/2] net/avf: fix unused variables and label
Date: Tue, 18 Sep 2018 15:18:50 +0100	[thread overview]
Message-ID: <20180918141850.GA16740@bricha3-MOBL.ger.corp.intel.com> (raw)
In-Reply-To: <7b1032a7-78e4-019e-ccba-3e1e5488bc56@intel.com>

On Tue, Sep 18, 2018 at 02:51:27PM +0100, Ferruh Yigit wrote:
> On 9/18/2018 2:17 PM, Bruce Richardson wrote:
> > Compiling with all warnings turned on causes errors about unused variables
> > and an unused label. Remove these to allow building without having to
> > disable those warnings.
> > 
> > Fixes: 69dd4c3d0898 ("net/avf: enable queue and device")
> > Fixes: 3fd7a3719c66 ("net/avf: enable ops for MTU setting")
> > Fixes: d6bde6b5eae9 ("net/avf: enable Rx interrupt")
> > Fixes: 22b123a36d07 ("net/avf: initialize PMD")
> > Fixes: 319c421f3890 ("net/avf: enable SSE Rx Tx")
> > Fixes: a2b29a7733ef ("net/avf: enable basic Rx Tx")>
> > CC: stable@dpdk.org
> > 
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > Acked-by: Luca Boccassi <bluca@debian.org>
> 
> <...>
> 
> > @@ -1268,7 +1266,6 @@ static inline uint16_t
> >  rx_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
> >  {
> >  	struct avf_rx_queue *rxq = (struct avf_rx_queue *)rx_queue;
> > -	struct rte_eth_dev *dev;
> 
> Fixes: 1060591eada5 ("net/avf: enable bulk allocate Rx")
> 
There's always one more! :-(

> >  	uint16_t nb_rx = 0;
> >  
> >  	if (!nb_pkts)
> > @@ -1584,10 +1581,6 @@ avf_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, uint16_t nb_pkts)
> >  
> >  		if (nb_ctx) {
> >  			/* Setup TX context descriptor if required */
> > -			volatile struct avf_tx_context_desc *ctx_txd =
> > -				(volatile struct avf_tx_context_desc *)
> > -					&txr[tx_id];
> 
> ctx_txd seems used in below macro controlled by DEBUG_DUMP_DESC define
> AVF_DUMP_TX_DESC(txq, ctx_txd, tx_id);

Yes, looking now it is. The function that is called from that macro takes a
void *, so I don't think the temporary variable is needed at all. I'll do a
V3, changing ctx_txd to &txr[tx_id] in that instance.

However, compiling with the DUMP_DESC flag turned on raises a whole set of
other compiler warnings in the code - even with 18.08 release. Therefore
that option should either be fixed and made a proper debug option in the
build config, or else dropped.

/Bruce