Re: [PATCH 00/12] net/xsc: Resolve issues from PVS and Coverity Scan

[Stephen Hemminger <stephen@networkplumber.org>]

On Sat, 22 Feb 2025 11:57:59 +0800
"Renyong Wan" <wanry@yunsilicon.com> wrote:

> This patch series resolves several issues reported by PVS and Coverity Scan,
> which were earlier forwarded to us by Stephen Hemminger.
> 
> ---
> Renyong Wan (12):
>   net/xsc: avoid integer overflow
>   net/xsc: remove useless call
>   net/xsc: address incorrect format warnings
>   net/xsc: remove always-true if expressions
>   net/xsc: avoid variable is assigned but not used
>   net/xsc: check possible null pointer dereference
>   net/xsc: avoid potential null pointer before used
>   net/xsc: remove always-true part of if expression
>   net/xsc: avoid assign the same value to a variable
>   net/xsc: avoid initialize by same function
>   net/xsc: optimize memcmp returns not 0 check
>   net/xsc: avoid pointer cast to unrelated class
> 
>  drivers/net/xsc/xsc_dev.c       |  2 +-
>  drivers/net/xsc/xsc_ethdev.c    | 35 ++++++++----
>  drivers/net/xsc/xsc_np.c        | 17 +++---
>  drivers/net/xsc/xsc_rx.c        | 31 ++++++-----
>  drivers/net/xsc/xsc_tx.c        |  7 +--
>  drivers/net/xsc/xsc_vfio.c      | 97 ++++++++++++++++++++-------------
>  drivers/net/xsc/xsc_vfio_mbox.c |  2 +-
>  7 files changed, 111 insertions(+), 80 deletions(-)
> 

Applied to next-net Great to see the Coverity issues fixed.
Ran PVS Studio on it, and there are three potential things that could be fixed later.

1. xsc_ethdev.c (838)
V1027	Pointer to an object of the 'rte_device' class is cast to unrelated 'rte_pci_device' class.

This is a generic issue in bus_pci_driver.h that can be suppressed there.

2. xsc_rx.c (351)
V522	There might be dereferencing of a potential null pointer 'rxq_data'.

Looking at the code, there are two loops over the rxq's the first one just
dereferences, and the second pass uses a helper function that could return NULL.
Why not just use do direct index in second one:

-- a/drivers/net/xsc/xsc_rx.c
+++ b/drivers/net/xsc/xsc_rx.c
@@ -347,7 +347,7 @@ xsc_rss_qp_create(struct xsc_ethdev_priv *priv, int port_id)
        rqn_base = rte_be_to_cpu_32(out->qpn_base) & 0xffffff;

        for (i = 0; i < priv->num_rq; i++) {
-               rxq_data = xsc_rxq_get(priv, i);
+               rxq_data = (*priv->rxqs)[i];
                rxq_data->wqes = rxq_data->rq_pas->addr;
                if (!xsc_dev_is_vf(xdev))
                        rxq_data->rq_db = (uint32_t *)((uint8_t *)xdev->bar_addr +


3. xsc_vfio_mbox.c (575)
V1048	The 'size' variable was assigned the same value.

This one is harmless, since both commands have same size, the tool
is just being annoying. Can suppress via comment

--- a/drivers/net/xsc/xsc_vfio_mbox.c
+++ b/drivers/net/xsc/xsc_vfio_mbox.c
@@ -572,7 +572,7 @@ xsc_vfio_mbox_init(struct xsc_dev *xdev)
        cmdq->req_lay = cmdq->req_mz->addr;

        snprintf(name, RTE_MEMZONE_NAMESIZE, "%s_cmd_cq", xdev->pci_dev->device.name);
-       size = (1 << XSC_CMDQ_DEPTH_LOG) * sizeof(struct xsc_cmdq_rsp_layout);
+       size = (1 << XSC_CMDQ_DEPTH_LOG) * sizeof(struct xsc_cmdq_rsp_layout); // -V1048
        cmdq->rsp_mz = rte_memzone_reserve_aligned(name,
                                                   size, SOCKET_ID_ANY,
                                                   RTE_MEMZONE_IOVA_CONTIG,