From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from dpdk.org (dpdk.org [92.243.14.124])
	by dpdk.space (Postfix) with ESMTP id BBCA2A0096
	for <public@inbox.dpdk.org>; Thu, 14 Mar 2019 16:44:45 +0100 (CET)
Received: from [92.243.14.124] (localhost [127.0.0.1])
	by dpdk.org (Postfix) with ESMTP id C55423772;
	Thu, 14 Mar 2019 16:44:44 +0100 (CET)
Received: from mail-pf1-f196.google.com (mail-pf1-f196.google.com
 [209.85.210.196]) by dpdk.org (Postfix) with ESMTP id 06DF93572
 for <dev@dpdk.org>; Thu, 14 Mar 2019 16:44:42 +0100 (CET)
Received: by mail-pf1-f196.google.com with SMTP id i20so4105039pfo.6
 for <dev@dpdk.org>; Thu, 14 Mar 2019 08:44:41 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=networkplumber-org.20150623.gappssmtp.com; s=20150623;
 h=date:from:to:cc:subject:message-id:in-reply-to:references
 :mime-version:content-transfer-encoding;
 bh=1O9ByvphlbQTwKKGHu5BfNnM5VJmQDWGOtEsuK6DUwg=;
 b=jMekdezD38mh0nOMTOesAA9hlgbyvLdgIpVan/cUi4ancTgYRjVHCWTMaBELiShRKg
 H7qDV4YPsk7IRAJU8JNWh15iCxz6soFr+7GPeQ3ifkcHnErd0MJlXf8Y6VCn4CIzDfER
 b6vvUg1MgUNV4ABohpB8CWj4n52Vd5Aia9h2k7idWbGJoiDHkCM75uVjAedwcXtRdpYH
 INEv36vPioDymPopOpx4MHXQHvjdqWH9snmr7JqbLs7cnLyA4iP+Uy5a7iZ9aXopXl2Y
 GuL9Lgb0BFNfArFIJWCK3SND+FZLCLY5KEqhGK10VGq1uChtpz4QrgTxc7xMUuguf3ZP
 Nofw==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=1e100.net; s=20161025;
 h=x-gm-message-state:date:from:to:cc:subject:message-id:in-reply-to
 :references:mime-version:content-transfer-encoding;
 bh=1O9ByvphlbQTwKKGHu5BfNnM5VJmQDWGOtEsuK6DUwg=;
 b=bbA3PZ6bgTUFaP0qav23EX85ozN4c+EAEBcFXk3UPdA7a6qCuN3I6kfL9z8icxSuGL
 tfPjRnnWOvb8wVia3R1igfl/lovt+Zyp8VzVzSBk7a1VYB5625kFTqMUnX/D24t6kOk9
 Bd+3y9qXe9hvm0NLe4S23p33UdcqQbnwwR3hp4EO5BCt92MG0rNcrVTlqrChDjJTL/hl
 5kspmmpdo7uEd3K4oqi4eMleRJx28WCOekdrObkFWTT/XD0+jjz1lR2MAoKXedwcm+Vt
 V5UyJxErQQG+Jlc5/o0W5M7bnqSIXsNpImlQl84q4GFDJ5Ol00sRlZC4k7IjCpB4tH9W
 idqQ==
X-Gm-Message-State: APjAAAWSRyLfZM3Mcs3ZAod4jQBnGZPOoZEdox8xQpo1JcSgdLJWQG22
 3R+30WH8HX+Lh1Kqul+TfNC3EvKuNDodGw==
X-Google-Smtp-Source: APXvYqzjL9H1PIkU4vaNbzsdkAtaCZvQx95SLw3bs5eWqI0Sc2wZl1nExr7N990NLS9Uq8g5G6bmkw==
X-Received: by 2002:aa7:9099:: with SMTP id i25mr50332886pfa.102.1552578281092; 
 Thu, 14 Mar 2019 08:44:41 -0700 (PDT)
Received: from shemminger-XPS-13-9360 (204-195-22-127.wavecable.com.
 [204.195.22.127])
 by smtp.gmail.com with ESMTPSA id k8sm17733768pgq.37.2019.03.14.08.44.40
 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);
 Thu, 14 Mar 2019 08:44:40 -0700 (PDT)
Date: Thu, 14 Mar 2019 08:44:35 -0700
From: Stephen Hemminger <stephen@networkplumber.org>
To: Matan Azrad <matan@mellanox.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>
Message-ID: <20190314084435.2cca9735@shemminger-XPS-13-9360>
In-Reply-To: <AM0PR0502MB4019F2341956C5FE4D7C409FD24B0@AM0PR0502MB4019.eurprd05.prod.outlook.com>
References: <20190313021253.525-1-stephen@networkplumber.org>
 <AM0PR0502MB4019F2341956C5FE4D7C409FD24B0@AM0PR0502MB4019.eurprd05.prod.outlook.com>
MIME-Version: 1.0
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
Subject: Re: [dpdk-dev] [PATCH] net/vdev_netvsc: fix erronous cast
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>
Message-ID: <20190314154435.zKElsFbptmzBI6EdO1TSWVZI8Ux8dzke9Y6g-M6o6AM@z>

On Thu, 14 Mar 2019 11:13:10 +0000
Matan Azrad <matan@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 <stephen@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.