DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] eventdev: prefix global variable
@ 2018-10-03 13:48 Ferruh Yigit
  2018-10-05  8:55 ` Ferruh Yigit
  0 siblings, 1 reply; 2+ messages in thread
From: Ferruh Yigit @ 2018-10-03 13:48 UTC (permalink / raw)
  To: Nikhil Rao, Jerin Jacob
  Cc: dev, Ferruh Yigit, stable, sunila.sahu, rasesh.mody, roy.fan.zhang

A global defined with name "stats".

DPDK is a library, please don't define global variables with this
generic name.

Fixes: b1ce8ebd97ba ("eventdev: add PMD callbacks for eth Rx adapter")
Cc: stable@dpdk.org

Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
There are already questions about that "stats" variable:
- Why a global variable defined in header file?
- It seems it is not used at all, remove it?
  Forgotten to be removed in below commit, but because of generic
  "stats" name, it didn't cause any compile error but happily used
  global variable
  Fixes: 3810ae435783 ("eventdev: add interrupt driven queues to Rx adapter")

But this patch mainly sent as an RFC, to ask what would you think about
a cleanup patch to add the library prefix for all the global variables
that are defined in libries? (yes we have lots of them)

Like:
zip_cc --> octeontx_zip_cc [1]
valid_args --> qede_valid_args [2]
scheduler --> crypto_scheduler [3]

[1]
Argh! This also looks wrong, I think intention was make this enum
name instead of global variable, can you please check. (This was random
sample I chosed.)
Commit 43e610bb8565 ("compress/octeontx: introduce octeontx zip PMD")
Cc: sunila.sahu@caviumnetworks.com

[2]
This is also wrong, it should be static,
Commit f64b91b0eb5d ("net/qede: replace config option with run-time arg")
Cc: rasesh.mody@cavium.com

[3]
This should be static too,
Commit 100e4f7e44ab ("crypto/scheduler: add round-robin mode")
Cc: roy.fan.zhang@intel.com
---
 lib/librte_eventdev/rte_event_eth_rx_adapter.c | 2 +-
 lib/librte_eventdev/rte_eventdev_pmd.h         | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/librte_eventdev/rte_event_eth_rx_adapter.c b/lib/librte_eventdev/rte_event_eth_rx_adapter.c
index 870ac8c3b..5d7d8435b 100644
--- a/lib/librte_eventdev/rte_event_eth_rx_adapter.c
+++ b/lib/librte_eventdev/rte_event_eth_rx_adapter.c
@@ -1125,7 +1125,7 @@ rxa_poll(struct rte_event_eth_rx_adapter *rx_adapter)
 	wrr_pos = rx_adapter->wrr_pos;
 	max_nb_rx = rx_adapter->max_nb_rx;
 	buf = &rx_adapter->event_enqueue_buffer;
-	stats = &rx_adapter->stats;
+	rte_eventdev_stats = &rx_adapter->stats;
 
 	/* Iterate through a WRR sequence */
 	for (num_queue = 0; num_queue < rx_adapter->wrr_len; num_queue++) {
diff --git a/lib/librte_eventdev/rte_eventdev_pmd.h b/lib/librte_eventdev/rte_eventdev_pmd.h
index 792fb3a23..14f9e4520 100644
--- a/lib/librte_eventdev/rte_eventdev_pmd.h
+++ b/lib/librte_eventdev/rte_eventdev_pmd.h
@@ -592,7 +592,7 @@ typedef int (*eventdev_eth_rx_adapter_stop_t)
 					(const struct rte_eventdev *dev,
 					const struct rte_eth_dev *eth_dev);
 
-struct rte_event_eth_rx_adapter_stats *stats;
+struct rte_event_eth_rx_adapter_stats *rte_eventdev_stats;
 
 /**
  * Retrieve ethernet Rx adapter statistics.
-- 
2.17.1

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [dpdk-dev] [PATCH] eventdev: prefix global variable
  2018-10-03 13:48 [dpdk-dev] [PATCH] eventdev: prefix global variable Ferruh Yigit
@ 2018-10-05  8:55 ` Ferruh Yigit
  0 siblings, 0 replies; 2+ messages in thread
From: Ferruh Yigit @ 2018-10-05  8:55 UTC (permalink / raw)
  To: Nikhil Rao, Jerin Jacob
  Cc: dev, stable, sunila.sahu, rasesh.mody, roy.fan.zhang

On 10/3/2018 2:48 PM, Ferruh Yigit wrote:
> A global defined with name "stats".
> 
> DPDK is a library, please don't define global variables with this
> generic name.
> 
> Fixes: b1ce8ebd97ba ("eventdev: add PMD callbacks for eth Rx adapter")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> ---
> There are already questions about that "stats" variable:
> - Why a global variable defined in header file?
> - It seems it is not used at all, remove it?
>   Forgotten to be removed in below commit, but because of generic
>   "stats" name, it didn't cause any compile error but happily used
>   global variable
>   Fixes: 3810ae435783 ("eventdev: add interrupt driven queues to Rx adapter")
> 
> But this patch mainly sent as an RFC, to ask what would you think about
> a cleanup patch to add the library prefix for all the global variables
> that are defined in libries? (yes we have lots of them)
> 
> Like:
> zip_cc --> octeontx_zip_cc [1]
> valid_args --> qede_valid_args [2]
> scheduler --> crypto_scheduler [3]

The cleanup patch has been sent:
"fix static variables"
https://patches.dpdk.org/patch/46006/

This patch has been superseded by above one.

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2018-10-05  8:55 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-03 13:48 [dpdk-dev] [PATCH] eventdev: prefix global variable Ferruh Yigit
2018-10-05  8:55 ` Ferruh Yigit

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).