From mboxrd@z Thu Jan  1 00:00:00 1970
From: Thomas Monjalon <thomas@monjalon.net>
Subject: Re: [PATCH 1/5] bus/vdev: add iteration filter on name
Date: Mon, 08 Oct 2018 09:47:27 +0200
Message-ID: <3036779.UqIVyZ4pAr@xps>
References: <20181007222554.4886-1-thomas@monjalon.net>
 <20181007222554.4886-2-thomas@monjalon.net>
 <bba84945-d5c5-285c-1e01-fa849c20b721@solarflare.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="iso-8859-1"
Content-Transfer-Encoding: quoted-printable
Cc: dev@dpdk.org, gaetan.rivet@6wind.com, ophirmu@mellanox.com,
 ferruh.yigit@intel.com
To: Andrew Rybchenko <arybchenko@solarflare.com>
Return-path: <dev-bounces@dpdk.org>
Received: from out3-smtp.messagingengine.com (out3-smtp.messagingengine.com
 [66.111.4.27]) by dpdk.org (Postfix) with ESMTP id B15B3160
 for <dev@dpdk.org>; Mon,  8 Oct 2018 09:47:31 +0200 (CEST)
In-Reply-To: <bba84945-d5c5-285c-1e01-fa849c20b721@solarflare.com>
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>

08/10/2018 08:46, Andrew Rybchenko:
> On 10/8/18 1:25 AM, Thomas Monjalon wrote:
> > A virtual device can be matched with following syntax:
> > 	bus=3Dvdev,name=3DX
> >
> > Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> > ---
> >   drivers/bus/vdev/vdev_params.c | 21 ++++++++++++++++++---
> >   1 file changed, 18 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/bus/vdev/vdev_params.c b/drivers/bus/vdev/vdev_par=
ams.c
> > index da270f2ec..133998c3e 100644
> > --- a/drivers/bus/vdev/vdev_params.c
> > +++ b/drivers/bus/vdev/vdev_params.c
> > @@ -2,6 +2,8 @@
> >    * Copyright 2018 Ga=EBtan Rivet
> >    */
> >  =20
> > +#include <string.h>
> > +
> >   #include <rte_dev.h>
> >   #include <rte_bus.h>
> >   #include <rte_kvargs.h>
> > @@ -11,10 +13,12 @@
> >   #include "vdev_private.h"
> >  =20
> >   enum vdev_params {
> > +	RTE_VDEV_PARAM_NAME,
> >   	RTE_VDEV_PARAM_MAX,
> >   };
> >  =20
> >   static const char * const vdev_params_keys[] =3D {
> > +	[RTE_VDEV_PARAM_NAME] =3D "name",
> >   	[RTE_VDEV_PARAM_MAX] =3D NULL,
> >   };
> >  =20
> > @@ -22,11 +26,22 @@ static int
> >   vdev_dev_match(const struct rte_device *dev,
> >   	       const void *_kvlist)
> >   {
> > +	int ret;
> >   	const struct rte_kvargs *kvlist =3D _kvlist;
> > +	char *name;
> > +
> > +	/* cannot pass const dev->name to rte_kvargs_process() */
> > +	name =3D strdup(dev->name);
> > +	if (name =3D=3D NULL)
> > +		return -ENOMEM; /* interpreted as no match */
>=20
> It is strange to see -ENOMEM and -1 returned from the same function.
> rte_dev_cmp_t does not return negative errno. It just says match /
> no match (greater / smaller than 0 if ordering is possible).
> So, -ENOMEM is really confusing here. I think just -1 should be used.

Yes, OK.

> > +	ret =3D rte_kvargs_process(kvlist,
> > +		vdev_params_keys[RTE_VDEV_PARAM_NAME],
> > +		rte_kvargs_strcmp, name);
> > +	free(name);
> > +	if (ret !=3D 0)
> > +		return -1;
> >  =20
> > -	(void) kvlist;
> > -	(void) dev;
> > -	return 0;
> > +	return ret;
>=20
> I'm not sure that I understand why 'ret' is returned here
> instead of 0. Above check guarantees that ret=3D=3D0.
> If you change it, it should be a good reason.

Right

Thanks for the review!