On 05/18/2018 06:59 PM, Shreyansh Jain wrote:
-----Original Message-----
From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Andy Green
Sent: Thursday, May 17, 2018 7:19 PM
To: dev@dpdk.org
Subject: [dpdk-dev] [PATCH v5 01/21] lib/librte_ethdev: change eth-dev-
ops API to return int
Signed-off-by: Andy Green <a...@warmcat.com>
---
drivers/net/ark/ark_ethdev_rx.c | 4 ++--
drivers/net/ark/ark_ethdev_rx.h | 3 +--
drivers/net/avf/avf_rxtx.c | 4 ++--
drivers/net/avf/avf_rxtx.h | 2 +-
drivers/net/bnxt/bnxt_ethdev.c | 5 +++--
drivers/net/dpaa/dpaa_ethdev.c | 4 ++--
drivers/net/dpaa2/dpaa2_ethdev.c | 6 +++---
drivers/net/e1000/e1000_ethdev.h | 6 ++----
drivers/net/e1000/em_rxtx.c | 4 ++--
drivers/net/e1000/igb_rxtx.c | 4 ++--
drivers/net/enic/enic_ethdev.c | 9 +++------
drivers/net/i40e/i40e_rxtx.c | 4 ++--
drivers/net/i40e/i40e_rxtx.h | 3 +--
drivers/net/ixgbe/ixgbe_ethdev.h | 3 +--
drivers/net/ixgbe/ixgbe_rxtx.c | 4 ++--
drivers/net/nfp/nfp_net.c | 9 ++++-----
drivers/net/sfc/sfc_ethdev.c | 4 ++--
drivers/net/thunderx/nicvf_ethdev.c | 2 +-
drivers/net/thunderx/nicvf_rxtx.c | 4 ++--
drivers/net/thunderx/nicvf_rxtx.h | 2 +-
drivers/net/vhost/rte_eth_vhost.c | 4 ++--
examples/l3fwd-power/main.c | 2 +-
lib/librte_ethdev/rte_ethdev_core.h | 4 ++--
23 files changed, 44 insertions(+), 52 deletions(-)
[...]
rxq = dev->data->rx_queues[rx_queue_id];
diff --git a/drivers/net/dpaa/dpaa_ethdev.c
b/drivers/net/dpaa/dpaa_ethdev.c
index d014a11aa..70a5b4851 100644
--- a/drivers/net/dpaa/dpaa_ethdev.c
+++ b/drivers/net/dpaa/dpaa_ethdev.c
@@ -725,7 +725,7 @@ static void dpaa_eth_tx_queue_release(void *txq
__rte_unused)
PMD_INIT_FUNC_TRACE();
}
-static uint32_t
+static int
dpaa_dev_rx_queue_count(struct rte_eth_dev *dev, uint16_t rx_queue_id)
{
struct dpaa_if *dpaa_intf = dev->data->dev_private;
@@ -738,7 +738,7 @@ dpaa_dev_rx_queue_count(struct rte_eth_dev *dev,
uint16_t rx_queue_id)
RTE_LOG(DEBUG, PMD, "RX frame count for q(%d) is %u\n",
rx_queue_id, frm_cnt);
}
- return frm_cnt;
+ return (int)frm_cnt;
}
static int dpaa_link_down(struct rte_eth_dev *dev)
diff --git a/drivers/net/dpaa2/dpaa2_ethdev.c
b/drivers/net/dpaa2/dpaa2_ethdev.c
index 9297725d9..eb6245b83 100644
--- a/drivers/net/dpaa2/dpaa2_ethdev.c
+++ b/drivers/net/dpaa2/dpaa2_ethdev.c
@@ -604,7 +604,7 @@ dpaa2_dev_tx_queue_release(void *q __rte_unused)
PMD_INIT_FUNC_TRACE();
}
-static uint32_t
+static int
dpaa2_dev_rx_queue_count(struct rte_eth_dev *dev, uint16_t
rx_queue_id)
{
int32_t ret;
@@ -612,7 +612,7 @@ dpaa2_dev_rx_queue_count(struct rte_eth_dev *dev,
uint16_t rx_queue_id)
struct dpaa2_queue *dpaa2_q;
struct qbman_swp *swp;
struct qbman_fq_query_np_rslt state;
- uint32_t frame_cnt = 0;
+ int frame_cnt = 0;
PMD_INIT_FUNC_TRACE();
@@ -628,7 +628,7 @@ dpaa2_dev_rx_queue_count(struct rte_eth_dev *dev,
uint16_t rx_queue_id)
dpaa2_q = (struct dpaa2_queue *)priv->rx_vq[rx_queue_id];
if (qbman_fq_query_state(swp, dpaa2_q->fqid, &state) == 0) {
- frame_cnt = qbman_fq_state_frame_count(&state);
+ frame_cnt = (int)qbman_fq_state_frame_count(&state);
DPAA2_PMD_DEBUG("RX frame count for q(%d) is %u",
rx_queue_id, frame_cnt);
}
This doesn't feel correct. A counter, especially the number of descriptors in a
queue, doesn't have a negative value. So, 1) this is an unnatural return and 2)
we litter the code with unnecessary typecast.
In fact, even in the above change, the debug messages continue to print
unsigned values. So, another typecast would be required there.
I don't agree with this change - at least not until some strong gcc 8 warning
reason is triggering this. Can you please point me to some conversation on
mailing list which enforces this?
hmmmmm.... no, it's not my idea.
If you don't like it, don't do it, I don't mind either way. I sent a
patch that just solved the compiler error only already, and was told on
the list it would be cooler if these things returned an int instead.
There's no point challenging me about the wisdom of it, although it
seems reasonable to me. I sent a patch, list guy $1 says do X instead,
I do X and then list guy $2 says EXPLAIN YOURSELF.
I really don't care, $1 should argue with $2 and leave me out of it.
If I don't hear anything more I'll reissue with just the minimal change
later.
-Andy
