Re: [dpdk-dev] [PATCH v4] ethdev: add sanity checks in control APIs

[Thomas Monjalon <thomas@monjalon.net>]

15/04/2021 14:04, Kevin Traynor:
> On 15/04/2021 01:52, Min Hu (Connor) wrote:
> > +	if (iter == NULL) {
> > +		RTE_ETHDEV_LOG(ERR, "Failed to iterator init for NULL\n");
> 
> "Failed to init iterator for NULL iterator\n"

The word "Failed" looks weird in these checks.
What about "Cannot"?
Example: "Cannot init NULL iterator"

> > +	if (devargs_str == NULL) {
> > +		RTE_ETHDEV_LOG(ERR, "Failed to iterate matching NULL\n");
> 
> "Failed to init iterator for NULL devargs\n"

"Cannot init iterator for NULL devargs"

> > +	if (owner == NULL) {
> > +		RTE_ETHDEV_LOG(ERR, "Failed to get ethdev port %u owner by NULL\n",
> > +			port_id);
> > +		return -EINVAL;
> > +	}
> 
> This fn uses both %u and %"PRIu16" for port_id

I don't see the benefit of PRIu16.

> > +	if (str == NULL) {
> > +		RTE_ETHDEV_LOG(ERR, "Failed to convert link to NULL\n");
> 
> "Failed to convert link to NULL string\n"

"Cannot convert link in NULL string"

> > +	if (eth_link == NULL) {
> > +		RTE_ETHDEV_LOG(ERR, "Failed to convert NULL to string\n");
> 
> "Failed to convert NULL link to string\n"

"Cannot convert NULL link"