[dpdk-dev] [PATCH v4 4/5] bus: add device arguments name parsing API

[Xueming(Steven) Li]

> -----Original Message-----
> From: Thomas Monjalon <thomas at monjalon.net>
> Sent: Tuesday, April 13, 2021 5:17 AM
> To: Xueming(Steven) Li <xuemingl at nvidia.com>
> Cc: Gaetan Rivet <gaetanr at nvidia.com>; dev at dpdk.org; Asaf Penso <asafp at nvidia.com>
> Subject: Re: [dpdk-dev] [PATCH v4 4/5] bus: add device arguments name parsing API
> 
> 10/04/2021 16:23, Xueming Li:
> > +	/* Resolve devarg's name. */
> 
> s/devarg's name/devargs name/
> 
> > +	if (bus && bus->devargs_parse)
> 
> Please make checks explicits with != NULL
> 
> > +		ret = bus->devargs_parse(devargs);
> > +	else if (layers[0].kvlist != NULL)
> > +		ret = devargs_bus_parse_default(devargs, layers[0].kvlist);
> [...]
> > +/**
> > + * Parse device arguments, setting the device name in the devargs as a result.
> 
> It should be
> "
> Parse bus part of the device arguments.
> 
> The field name of the struct rte_devargs will be set.
> "
> 
> > + *
> > + * On error rte_errno is set.
> 
> This sentence can be below  (in @return section).
> 
> > + *
> > + * @param da
> > + *	Pointer to the devargs to parse.
> > + *	The 'bus_str' field must be set.
> 
> Why "must"?
> It should be optional, so this sentence should be removed.
> 
> > + *
> > + * @return
> > + *	0 on successful parsing.
> > + *	-EINVAL: on parsing error.
> > + *	-ENODEV: if no key matching a device argument is specified.
> > + *	-E2BIG: device name is too long.
> > + */
> > +typedef int (*rte_bus_devargs_parse_t)(struct rte_devargs *da);
> 
> [...]
> > +	rte_bus_devargs_parse_t devargs_parse; /**< Parse device arguments */
> 
> Should be "Parse bus devargs"

Thanks, will fix all issues in next version.
> 
>