On Thu, 14 Mar 2019 11:13:10 +0000
Matan Azrad <ma...@mellanox.com> wrote:
> Hi
>
> From: Stephen Hemminger
> > The return value from bus->find_device is a rte_device which is not safe to
> > cast to a rte_vdev_device structure.
> > It doesn't really matter since only being checked for NULL but static
> > checkers
> > might find a bug here.
> >
>
> Fix line is missing.
>
> > Signed-off-by: Stephen Hemminger <step...@networkplumber.org>
> > ---
> > drivers/net/vdev_netvsc/vdev_netvsc.c | 7 ++++---
> > 1 file changed, 4 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/net/vdev_netvsc/vdev_netvsc.c
> > b/drivers/net/vdev_netvsc/vdev_netvsc.c
> > index ba63fac2a598..801f54c96e01 100644
> > --- a/drivers/net/vdev_netvsc/vdev_netvsc.c
> > +++ b/drivers/net/vdev_netvsc/vdev_netvsc.c
> > @@ -808,7 +808,7 @@ vdev_netvsc_cmp_rte_device(const struct
> > rte_device *dev1, static void vdev_netvsc_scan_callback(__rte_unused
> > void *arg) {
> > - struct rte_vdev_device *dev;
> > + struct rte_device *dev;
> > struct rte_devargs *devargs;
> > struct rte_bus *vbus = rte_bus_find_by_name("vdev");
> >
> > @@ -816,8 +816,9 @@ vdev_netvsc_scan_callback(__rte_unused void *arg)
> > if (!strncmp(devargs->name, VDEV_NETVSC_DRIVER_NAME,
> > VDEV_NETVSC_DRIVER_NAME_LEN))
> > return;
> > - dev = (struct rte_vdev_device *)vbus->find_device(NULL,
> > - vdev_netvsc_cmp_rte_device,
> > VDEV_NETVSC_DRIVER_NAME);
> > +
> > + dev = vbus->find_device(NULL, vdev_netvsc_cmp_rte_device,
> > + VDEV_NETVSC_DRIVER_NAME);
>
> Since the device must be vdev here,
> It is better to use explicit cast to make the checker happy.
The cast is converting from rte_device to rte_vdev_device which incorrect.
The device returned by vbus->find_device is a rte_device not a rte_vdev_device:
vbus->find_device points to rte_vdev_find_device
struct rte_device *
rte_vdev_find_device(const struct rte_device *start, rte_dev_cmp_t cmp,
const void *data)
but definition of rte_vdev_device is:
struct rte_vdev_device {
TAILQ_ENTRY(rte_vdev_device) next; /**< Next attached vdev */
struct rte_device device; /**< Inherit core device */
};
The only safe way to get rte_vdev_device would be to use container_of().
dev = vbus->find_device(NULL, vdev_netvsc_cmp_rte_device,
VDEV_NETVSC_DRIVER_NAME);
vdev = dev ? container_of(dev, struct rte_vdev_device, device) : NULL;
But as the PATCH demonstrated, this is not necessary.
