net/avf: remove unused variables and label

Message ID 20180913144405.72772-1-bruce.richardson@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Qi Zhang
Headers
Series net/avf: remove unused variables and label |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Bruce Richardson Sept. 13, 2018, 2:44 p.m. UTC
  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.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 drivers/net/avf/avf_ethdev.c | 15 +--------------
 drivers/net/avf/avf_rxtx.c   | 17 +++++------------
 drivers/net/avf/avf_vchnl.c  |  2 --
 3 files changed, 6 insertions(+), 28 deletions(-)
  

Comments

Luca Boccassi Sept. 13, 2018, 4:26 p.m. UTC | #1
On Thu, 2018-09-13 at 15:44 +0100, 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.
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
>  drivers/net/avf/avf_ethdev.c | 15 +--------------
>  drivers/net/avf/avf_rxtx.c   | 17 +++++------------
>  drivers/net/avf/avf_vchnl.c  |  2 --
>  3 files changed, 6 insertions(+), 28 deletions(-)

Acked-by: Luca Boccassi <bluca@debian.org>
  
Qi Zhang Sept. 17, 2018, 7:23 a.m. UTC | #2
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Luca Boccassi
> Sent: Friday, September 14, 2018 12:26 AM
> To: Richardson, Bruce <bruce.richardson@intel.com>; Wu, Jingjing
> <jingjing.wu@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] net/avf: remove unused variables and label
> 
> On Thu, 2018-09-13 at 15:44 +0100, 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.
> >
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > ---
> >  drivers/net/avf/avf_ethdev.c | 15 +--------------
> >  drivers/net/avf/avf_rxtx.c   | 17 +++++------------
> >  drivers/net/avf/avf_vchnl.c  |  2 --
> >  3 files changed, 6 insertions(+), 28 deletions(-)
> 
> Acked-by: Luca Boccassi <bluca@debian.org>
> 
> --
> Kind regards,
> Luca Boccassi

Applied to dpdk-next-net-intel.

Thanks
Qi
  
Ferruh Yigit Sept. 17, 2018, 2:53 p.m. UTC | #3
On 9/13/2018 3:44 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.

If this is fixing some build errors should it be backported?

And why we didn't get those warnings until now, aren't we already compiling with
all warnings turned on?

> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>

<...>
  
Bruce Richardson Sept. 17, 2018, 3:20 p.m. UTC | #4
On Mon, Sep 17, 2018 at 03:53:40PM +0100, Ferruh Yigit wrote:
> On 9/13/2018 3:44 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.
> 
> If this is fixing some build errors should it be backported?
> 
> And why we didn't get those warnings until now, aren't we already compiling with
> all warnings turned on?
> 
> > 
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> 
It does not appear so. When we add support for building this driver with
meson, the compiler warnings then appeared [See
http://patches.dpdk.org/patch/44698/]. 

While you can backport I'm not sure it's worthwhile, unless you plan on
backporting the new meson support too.  The warnings don't come from
external header files that apps would be using directly with unknown
compiler flags, but are internal to DPDK and don't seem to cause any issues
with "make" builds.

Regards,
/Bruce
  
Ferruh Yigit Sept. 17, 2018, 4:12 p.m. UTC | #5
On 9/17/2018 4:20 PM, Bruce Richardson wrote:
> On Mon, Sep 17, 2018 at 03:53:40PM +0100, Ferruh Yigit wrote:
>> On 9/13/2018 3:44 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.
>>
>> If this is fixing some build errors should it be backported?
>>
>> And why we didn't get those warnings until now, aren't we already compiling with
>> all warnings turned on?
>>
>>>
>>> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
>>
> It does not appear so. When we add support for building this driver with
> meson, the compiler warnings then appeared [See
> http://patches.dpdk.org/patch/44698/]. 
> 
> While you can backport I'm not sure it's worthwhile, unless you plan on
> backporting the new meson support too.  The warnings don't come from
> external header files that apps would be using directly with unknown
> compiler flags, but are internal to DPDK and don't seem to cause any issues
> with "make" builds.

Argh, it is missing warning flags in Makefile:
 +CFLAGS += $(WERROR_FLAGS)

When above added "make" also gives "unused variable" warnings.

Even we don't observe the issue with current makefile, if someone builds with:
'make  EXTRA_CFLAGS="-Wunused-variable"' will observe the warnings.

That is why I am for backporting this patch, does it make sense?

And would you mind adding above CFLAGS update into this patch?
  
Bruce Richardson Sept. 17, 2018, 4:24 p.m. UTC | #6
On Mon, Sep 17, 2018 at 05:12:19PM +0100, Ferruh Yigit wrote:
> On 9/17/2018 4:20 PM, Bruce Richardson wrote:
> > On Mon, Sep 17, 2018 at 03:53:40PM +0100, Ferruh Yigit wrote:
> >> On 9/13/2018 3:44 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.
> >>
> >> If this is fixing some build errors should it be backported?
> >>
> >> And why we didn't get those warnings until now, aren't we already compiling with
> >> all warnings turned on?
> >>
> >>>
> >>> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> >>
> > It does not appear so. When we add support for building this driver with
> > meson, the compiler warnings then appeared [See
> > http://patches.dpdk.org/patch/44698/]. 
> > 
> > While you can backport I'm not sure it's worthwhile, unless you plan on
> > backporting the new meson support too.  The warnings don't come from
> > external header files that apps would be using directly with unknown
> > compiler flags, but are internal to DPDK and don't seem to cause any issues
> > with "make" builds.
> 
> Argh, it is missing warning flags in Makefile:
>  +CFLAGS += $(WERROR_FLAGS)
> 
> When above added "make" also gives "unused variable" warnings.
> 
> Even we don't observe the issue with current makefile, if someone builds with:
> 'make  EXTRA_CFLAGS="-Wunused-variable"' will observe the warnings.
> 
> That is why I am for backporting this patch, does it make sense?
> 
> And would you mind adding above CFLAGS update into this patch?

Ok, I'll do up a V2.
  

Patch

diff --git a/drivers/net/avf/avf_ethdev.c b/drivers/net/avf/avf_ethdev.c
index 3a2baaf28..be9f163be 100644
--- a/drivers/net/avf/avf_ethdev.c
+++ b/drivers/net/avf/avf_ethdev.c
@@ -154,7 +154,6 @@  static int
 avf_init_rss(struct avf_adapter *adapter)
 {
 	struct avf_info *vf =  AVF_DEV_PRIVATE_TO_VF(adapter);
-	struct avf_hw *hw = AVF_DEV_PRIVATE_TO_HW(adapter);
 	struct rte_eth_rss_conf *rss_conf;
 	uint8_t i, j, nb_q;
 	int ret;
@@ -259,11 +258,8 @@  avf_init_rxq(struct rte_eth_dev *dev, struct avf_rx_queue *rxq)
 static int
 avf_init_queues(struct rte_eth_dev *dev)
 {
-	struct avf_info *vf = AVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
 	struct avf_rx_queue **rxq =
 		(struct avf_rx_queue **)dev->data->rx_queues;
-	struct avf_tx_queue **txq =
-		(struct avf_tx_queue **)dev->data->tx_queues;
 	int i, ret = AVF_SUCCESS;
 
 	for (i = 0; i < dev->data->nb_rx_queues; i++) {
@@ -415,7 +411,6 @@  avf_dev_start(struct rte_eth_dev *dev)
 		AVF_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
 	struct avf_info *vf = AVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
 	struct avf_hw *hw = AVF_DEV_PRIVATE_TO_HW(dev->data->dev_private);
-	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
 	struct rte_intr_handle *intr_handle = dev->intr_handle;
 
 	PMD_INIT_FUNC_TRACE();
@@ -476,9 +471,7 @@  avf_dev_stop(struct rte_eth_dev *dev)
 	struct avf_adapter *adapter =
 		AVF_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
 	struct avf_hw *hw = AVF_DEV_PRIVATE_TO_HW(dev->data->dev_private);
-	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
 	struct rte_intr_handle *intr_handle = dev->intr_handle;
-	int ret, i;
 
 	PMD_INIT_FUNC_TRACE();
 
@@ -503,8 +496,6 @@  avf_dev_stop(struct rte_eth_dev *dev)
 static void
 avf_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 {
-	struct avf_adapter *adapter =
-		AVF_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
 	struct avf_info *vf = AVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
 
 	memset(dev_info, 0, sizeof(*dev_info));
@@ -915,7 +906,6 @@  avf_dev_rss_hash_conf_get(struct rte_eth_dev *dev,
 static int
 avf_dev_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
 {
-	struct avf_info *vf = AVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
 	uint32_t frame_size = mtu + AVF_ETH_OVERHEAD;
 	int ret = 0;
 
@@ -1045,8 +1035,6 @@  avf_dev_rx_queue_intr_enable(struct rte_eth_dev *dev, uint16_t queue_id)
 static int
 avf_dev_rx_queue_intr_disable(struct rte_eth_dev *dev, uint16_t queue_id)
 {
-	struct avf_adapter *adapter =
-		AVF_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
 	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
 	struct avf_hw *hw = AVF_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 	uint16_t msix_intr;
@@ -1089,7 +1077,7 @@  avf_check_vf_reset_done(struct avf_hw *hw)
 static int
 avf_init_vf(struct rte_eth_dev *dev)
 {
-	int i, err, bufsz;
+	int err, bufsz;
 	struct avf_adapter *adapter =
 		AVF_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
 	struct avf_hw *hw = AVF_DEV_PRIVATE_TO_HW(dev->data->dev_private);
@@ -1198,7 +1186,6 @@  avf_dev_interrupt_handler(void *param)
 
 	avf_handle_virtchnl_msg(dev);
 
-done:
 	avf_enable_irq0(hw);
 }
 
diff --git a/drivers/net/avf/avf_rxtx.c b/drivers/net/avf/avf_rxtx.c
index e03a136fc..edff48d27 100644
--- a/drivers/net/avf/avf_rxtx.c
+++ b/drivers/net/avf/avf_rxtx.c
@@ -247,7 +247,6 @@  alloc_rxq_mbufs(struct avf_rx_queue *rxq)
 static inline void
 release_rxq_mbufs(struct avf_rx_queue *rxq)
 {
-	struct rte_mbuf *mbuf;
 	uint16_t i;
 
 	if (!rxq->sw_ring)
@@ -310,9 +309,8 @@  avf_dev_rx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_idx,
 	struct avf_rx_queue *rxq;
 	const struct rte_memzone *mz;
 	uint32_t ring_size;
-	uint16_t len, i;
+	uint16_t len;
 	uint16_t rx_free_thresh;
-	uint16_t base, bsf, tc_mapping;
 
 	PMD_INIT_FUNC_TRACE();
 
@@ -428,13 +426,10 @@  avf_dev_tx_queue_setup(struct rte_eth_dev *dev,
 		       const struct rte_eth_txconf *tx_conf)
 {
 	struct avf_hw *hw = AVF_DEV_PRIVATE_TO_HW(dev->data->dev_private);
-	struct avf_adapter *ad =
-		AVF_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
 	struct avf_tx_queue *txq;
 	const struct rte_memzone *mz;
 	uint32_t ring_size;
 	uint16_t tx_rs_thresh, tx_free_thresh;
-	uint16_t i, base, bsf, tc_mapping;
 	uint64_t offloads;
 
 	PMD_INIT_FUNC_TRACE();
@@ -515,8 +510,11 @@  avf_dev_tx_queue_setup(struct rte_eth_dev *dev,
 	txq->ops = &def_txq_ops;
 
 #ifdef RTE_LIBRTE_AVF_INC_VECTOR
-	if (check_tx_vec_allow(txq) == FALSE)
+	if (check_tx_vec_allow(txq) == FALSE) {
+		struct avf_adapter *ad =
+			AVF_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
 		ad->tx_vec_allowed = false;
+	}
 #endif
 
 	return 0;
@@ -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;
 	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];
-			uint16_t cd_l2tag2 = 0;
 			uint64_t cd_type_cmd_tso_mss =
 				AVF_TX_DESC_DTYPE_CONTEXT;
 
diff --git a/drivers/net/avf/avf_vchnl.c b/drivers/net/avf/avf_vchnl.c
index fa71014e1..fd90cc2c3 100644
--- a/drivers/net/avf/avf_vchnl.c
+++ b/drivers/net/avf/avf_vchnl.c
@@ -69,7 +69,6 @@  avf_execute_vf_cmd(struct avf_adapter *adapter, struct avf_cmd_info *args)
 {
 	struct avf_hw *hw = AVF_DEV_PRIVATE_TO_HW(adapter);
 	struct avf_info *vf = AVF_DEV_PRIVATE_TO_VF(adapter);
-	struct avf_arq_event_info event_info;
 	enum avf_status_code ret;
 	int err = 0;
 	int i = 0;
@@ -600,7 +599,6 @@  avf_config_irq_map(struct avf_adapter *adapter)
 	struct virtchnl_irq_map_info *map_info;
 	struct virtchnl_vector_map *vecmap;
 	struct avf_cmd_info args;
-	uint32_t vector_id;
 	int len, i, err;
 
 	len = sizeof(struct virtchnl_irq_map_info) +