All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Monjalon <thomas.monjalon@6wind.com>
To: Jan Viktorin <viktorin@rehivetech.com>
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev,1/2] ethdev: remove useless null checks
Date: Fri, 22 Jan 2016 10:11:47 +0100	[thread overview]
Message-ID: <14115211.gcWZQ474Ox@xps13> (raw)
In-Reply-To: <20160121200225.60cffcfd@pcviktorin.fit.vutbr.cz>

2016-01-21 20:02, Jan Viktorin:
> On Thu, 21 Jan 2016 12:57:10 +0100
> David Marchand <david.marchand@6wind.com> wrote:
> > -	if ((name == NULL) || (pci_dev == NULL))
> > -		return -EINVAL;
> 
> Do you use a kind of assert in DPDK? The patch looks OK, however, I
> would prefer something like
> 
> 	assert_not_null(name);
> 	assert_not_null(pci_dev);
> 
> Usually, if some outer code is broken by mistake, the assert catches
> such an issue. At the same time, it documents the code by telling
> "this must never be NULL here". I agree, that returning -EINVAL for
> this kind of check is incorrect.
> 
> Same for other changes...

For this patch, I agree to remove useless checks.
For the other checks, EINVAL seems to be the right error code.
And yes you are right, we could replace most of EINVAL returns by a kind
of assert. RTE_VERIFY would be appropriate.

  reply	other threads:[~2016-01-22  9:12 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-21 11:57 [PATCH 0/2] minor cleanup in ethdev hotplug David Marchand
2016-01-21 11:57 ` [PATCH 1/2] ethdev: remove useless null checks David Marchand
2016-01-21 19:02   ` [dpdk-dev,1/2] " Jan Viktorin
2016-01-22  9:11     ` Thomas Monjalon [this message]
2016-01-21 11:57 ` [PATCH 2/2] ethdev: move code to common place in hotplug David Marchand
2016-01-21 15:38   ` [dpdk-dev, " Jan Viktorin
2016-01-21 18:06     ` David Marchand
2016-01-21 18:42       ` Thomas Monjalon
2016-01-22  7:15         ` David Marchand
2016-01-22 14:06 ` [PATCH v2 0/2] minor cleanup in ethdev hotplug David Marchand
2016-01-22 14:06   ` [PATCH v2 1/2] ethdev: remove useless null checks David Marchand
2016-01-26 15:50     ` Jan Viktorin
2016-01-27  9:40       ` David Marchand
2016-01-22 14:06   ` [PATCH v2 2/2] ethdev: move code to common place in hotplug David Marchand
2016-01-26 15:48     ` Jan Viktorin
2016-01-27 15:10   ` [PATCH v2 0/2] minor cleanup in ethdev hotplug Thomas Monjalon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=14115211.gcWZQ474Ox@xps13 \
    --to=thomas.monjalon@6wind.com \
    --cc=dev@dpdk.org \
    --cc=viktorin@rehivetech.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.