> -----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?

Reply via email to