DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Gaëtan Rivet" <gaetan.rivet@6wind.com>
To: Stephen Hemminger <stephen@networkplumber.org>
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH v6 09/11] pci: implement find_device bus operation
Date: Wed, 28 Jun 2017 11:17:46 +0200	[thread overview]
Message-ID: <20170628091746.GG13355@bidouze.vm.6wind.com> (raw)
In-Reply-To: <20170627163514.2ed2ec6b@xeon-e3>

On Tue, Jun 27, 2017 at 04:35:14PM -0700, Stephen Hemminger wrote:
> On Tue, 27 Jun 2017 18:11:16 +0200
> Gaetan Rivet <gaetan.rivet@6wind.com> wrote:
> 
> > +	int start_found = !!(start == NULL);
> > +
> > +	FOREACH_DEVICE_ON_PCIBUS(dev) {
> > +		if (!start_found) {
> > +			if (&dev->device == start)
> > +				start_found = 1;
> > +			continue;
> > +		}
> > +		if (cmp(&dev->device, data) == 0)
> > +			return &dev->device;
> > +	}
> > +	return NULL;
> > +}
> > +
> 
> Why is start_found not a boolean?
> 

Ah, yes, I wrote this a few times over in rte_bus and rte_vdev, and
mostly used the same scheme in the PCI implementation, without checking
for the use of stdbool in the vincinity otherwise.

I would not be opposed to using a bool if I was rewriting it, but I
don't feel this change warrants a new version.

> Do you really need start_found at all? Why not just reuse existing
> pointer?
> 

You are right, it could be reduced. But I find it a little too "clever"
in a sense, and I prefer usually to avoid rewriting input parameters. If this
function had to be refactored later, the writer would need to be careful
about start having changed. The advantage of using one variable less
does not outweight this drawback I think.

> 	FOREACH_DEVICE_ON_PCIBUS(dev) {
> 		if (start) {
> 			if (&dev->device == start)
> 				start = NULL
> 			continue;
> 		}
> 		if (cmp(&dev->device, data) == 0)
> 			return &dev->device;
> 	}
> 	return NULL;
> }

-- 
Gaëtan Rivet
6WIND

  reply	other threads:[~2017-06-28  9:17 UTC|newest]

Thread overview: 117+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-06-26  0:21 [dpdk-dev] [PATCH v5 00/12] bus: attach / detach API Gaetan Rivet
2017-06-26  0:21 ` [dpdk-dev] [PATCH v5 01/12] bus: add bus iterator to find a bus Gaetan Rivet
2017-06-26 15:30   ` Bruce Richardson
2017-06-26 20:53     ` Gaëtan Rivet
2017-06-26  0:22 ` [dpdk-dev] [PATCH v5 02/12] bus: add device iterator method Gaetan Rivet
2017-06-26 16:20   ` Bruce Richardson
2017-06-26 21:13     ` Gaëtan Rivet
2017-06-26  0:22 ` [dpdk-dev] [PATCH v5 03/12] bus: add helper to find which bus holds a device Gaetan Rivet
2017-06-26 16:31   ` Bruce Richardson
2017-06-26 22:16     ` Gaëtan Rivet
2017-06-26  0:22 ` [dpdk-dev] [PATCH v5 04/12] bus: add bus iterator to find " Gaetan Rivet
2017-06-27 10:14   ` Bruce Richardson
2017-06-27 13:54   ` Bruce Richardson
2017-06-27 15:05     ` Gaëtan Rivet
2017-06-27 15:08       ` Bruce Richardson
2017-06-26  0:22 ` [dpdk-dev] [PATCH v5 05/12] bus: introduce hotplug functionality Gaetan Rivet
2017-06-27 12:23   ` Bruce Richardson
2017-06-26  0:22 ` [dpdk-dev] [PATCH v5 06/12] vdev: implement find_device bus operation Gaetan Rivet
2017-06-27 12:25   ` Bruce Richardson
2017-06-26  0:22 ` [dpdk-dev] [PATCH v5 07/12] vdev: implement hotplug functionality Gaetan Rivet
2017-06-27 12:41   ` Bruce Richardson
2017-06-26  0:22 ` [dpdk-dev] [PATCH v5 08/12] vdev: expose bus name Gaetan Rivet
2017-06-27 12:45   ` Bruce Richardson
2017-06-26  0:22 ` [dpdk-dev] [PATCH v5 09/12] vdev: use standard bus registration function Gaetan Rivet
2017-06-27 12:59   ` Bruce Richardson
2017-06-26  0:22 ` [dpdk-dev] [PATCH v5 10/12] pci: implement find_device bus operation Gaetan Rivet
2017-06-27 13:36   ` Bruce Richardson
2017-06-26  0:22 ` [dpdk-dev] [PATCH v5 11/12] pci: implement hotplug " Gaetan Rivet
2017-06-27 13:49   ` Bruce Richardson
2017-06-26  0:22 ` [dpdk-dev] [PATCH v5 12/12] eal: make virtual driver probe and remove take rte_vdev_device Gaetan Rivet
2017-06-27 13:58   ` Bruce Richardson
2017-06-27 14:47     ` Gaëtan Rivet
2017-06-27 15:06       ` Bruce Richardson
2017-06-27 14:00   ` Bruce Richardson
2017-06-27 14:08 ` [dpdk-dev] [PATCH v5 00/12] bus: attach / detach API Bruce Richardson
2017-06-27 14:48   ` Gaëtan Rivet
2017-06-27 16:11 ` [dpdk-dev] [PATCH v6 00/11] " Gaetan Rivet
2017-06-27 16:11   ` [dpdk-dev] [PATCH v6 01/11] bus: add bus iterator to find a bus Gaetan Rivet
2017-06-27 16:11   ` [dpdk-dev] [PATCH v6 02/11] bus: add device iterator method Gaetan Rivet
2017-06-27 16:11   ` [dpdk-dev] [PATCH v6 03/11] bus: add helper to find which bus holds a device Gaetan Rivet
2017-06-27 16:11   ` [dpdk-dev] [PATCH v6 04/11] bus: add bus iterator to find " Gaetan Rivet
2017-06-27 16:11   ` [dpdk-dev] [PATCH v6 05/11] bus: introduce hotplug functionality Gaetan Rivet
2017-06-27 19:03     ` Jan Blunck
2017-06-28 11:44       ` Thomas Monjalon
2017-06-28 11:58         ` Jan Blunck
2017-06-28 12:11           ` Thomas Monjalon
2017-06-28 13:09             ` Jan Blunck
2017-06-28 13:30               ` Thomas Monjalon
2017-06-28 15:11                 ` Jan Blunck
2017-06-28 15:33                   ` Bruce Richardson
2017-06-29 12:59                     ` Gaëtan Rivet
2017-06-29 19:20                       ` Jan Blunck
2017-06-30 11:32                         ` Gaëtan Rivet
2017-06-27 16:11   ` [dpdk-dev] [PATCH v6 06/11] vdev: implement find_device bus operation Gaetan Rivet
2017-06-27 16:11   ` [dpdk-dev] [PATCH v6 07/11] vdev: implement hotplug " Gaetan Rivet
2017-06-27 16:11   ` [dpdk-dev] [PATCH v6 08/11] vdev: use standard bus registration function Gaetan Rivet
2017-07-03 22:15     ` Thomas Monjalon
2017-06-27 16:11   ` [dpdk-dev] [PATCH v6 09/11] pci: implement find_device bus operation Gaetan Rivet
2017-06-27 23:35     ` Stephen Hemminger
2017-06-28  9:17       ` Gaëtan Rivet [this message]
2017-06-28  9:52         ` Richardson, Bruce
2017-06-27 16:11   ` [dpdk-dev] [PATCH v6 10/11] pci: implement hotplug " Gaetan Rivet
2017-06-27 16:11   ` [dpdk-dev] [PATCH v6 11/11] dev: use new hotplug API in attach / detach Gaetan Rivet
2017-06-29 18:21   ` [dpdk-dev] [PATCH 00/15] bus attach/detach API and hotplug rework Jan Blunck
2017-06-29 18:21     ` [dpdk-dev] [PATCH v7 01/15] bus: add bus iterator to find a bus Jan Blunck
2017-06-29 18:21     ` [dpdk-dev] [PATCH v7 02/15] bus: add find_device bus operation Jan Blunck
2017-06-29 18:21     ` [dpdk-dev] [PATCH v7 03/15] vdev: implement " Jan Blunck
2017-06-29 18:21     ` [dpdk-dev] [PATCH v7 04/15] pci: " Jan Blunck
2017-06-29 18:21     ` [dpdk-dev] [PATCH v7 05/15] bus/fslmc: " Jan Blunck
2017-06-29 18:21     ` [dpdk-dev] [PATCH v7 06/15] bus: add helper to find which bus holds a device Jan Blunck
2017-06-30  9:16       ` Thomas Monjalon
2017-06-30 16:46         ` Jan Blunck
2017-06-30 18:29           ` Thomas Monjalon
2017-06-30 21:24             ` Bruce Richardson
2017-06-30 21:25           ` Bruce Richardson
2017-06-29 18:21     ` [dpdk-dev] [PATCH v7 07/15] bus: add bus iterator to find " Jan Blunck
2017-06-30  9:17       ` Thomas Monjalon
2017-06-29 18:21     ` [dpdk-dev] [PATCH v7 08/15] bus: require buses to implement find_device operation Jan Blunck
2017-06-29 18:22     ` [dpdk-dev] [PATCH v7 09/15] bus: add rte_bus_find_by_name Jan Blunck
2017-06-29 18:22     ` [dpdk-dev] [PATCH v7 10/15] bus: introduce device plug/unplug functionality Jan Blunck
2017-06-29 18:22     ` [dpdk-dev] [PATCH v7 11/15] vdev: implement unplug bus operation Jan Blunck
2017-06-29 18:22     ` [dpdk-dev] [PATCH v7 12/15] pci: implement hotplug " Jan Blunck
2017-06-29 18:22     ` [dpdk-dev] [PATCH v7 13/15] eal: add hotplug add/remove functions Jan Blunck
2017-06-30  9:06       ` Thomas Monjalon
2017-06-30  9:11         ` Gaëtan Rivet
2017-06-30  9:20           ` Bruce Richardson
2017-06-30 15:44             ` Jan Blunck
2017-06-30 16:03               ` Thomas Monjalon
2017-06-30 16:13                 ` Gaëtan Rivet
2017-06-30 16:25                   ` Bruce Richardson
2017-06-30 16:29                     ` Thomas Monjalon
2017-06-30 12:54       ` Thomas Monjalon
2017-06-30 15:12         ` Jan Blunck
2017-06-29 18:22     ` [dpdk-dev] [PATCH v7 14/15] dev: use new hotplug API in attach / detach Jan Blunck
2017-06-29 21:05       ` Thomas Monjalon
2017-06-29 18:22     ` [dpdk-dev] [PATCH v7 15/15] ethdev: Use embedded rte_device to detach driver Jan Blunck
2017-06-29 20:58       ` Thomas Monjalon
2017-06-30  9:59     ` [dpdk-dev] [PATCH 00/15] bus attach/detach API and hotplug rework Thomas Monjalon
2017-06-30 18:19     ` [dpdk-dev] [PATCH v8 00/14] " Jan Blunck
2017-06-30 18:19       ` [dpdk-dev] [PATCH v8 01/14] bus: add bus iterator to find a bus Jan Blunck
2017-06-30 18:19       ` [dpdk-dev] [PATCH v8 02/14] bus: add find_device bus operation Jan Blunck
2017-06-30 18:19       ` [dpdk-dev] [PATCH v8 03/14] vdev: implement " Jan Blunck
2017-06-30 18:19       ` [dpdk-dev] [PATCH v8 04/14] pci: " Jan Blunck
2017-06-30 18:19       ` [dpdk-dev] [PATCH v8 05/14] bus/fslmc: " Jan Blunck
2017-06-30 18:19       ` [dpdk-dev] [PATCH v8 06/14] bus: add helper to find which bus holds a device Jan Blunck
2017-06-30 18:19       ` [dpdk-dev] [PATCH v8 07/14] bus: require buses to implement find_device operation Jan Blunck
2017-06-30 18:19       ` [dpdk-dev] [PATCH v8 08/14] bus: add rte_bus_find_by_name Jan Blunck
2017-06-30 18:19       ` [dpdk-dev] [PATCH v8 09/14] bus: introduce device plug/unplug functionality Jan Blunck
2017-06-30 18:38         ` Gaëtan Rivet
2017-06-30 18:52           ` Jan Blunck
2017-06-30 18:19       ` [dpdk-dev] [PATCH v8 10/14] vdev: implement unplug bus operation Jan Blunck
2017-06-30 18:19       ` [dpdk-dev] [PATCH v8 11/14] pci: implement hotplug " Jan Blunck
2017-06-30 18:19       ` [dpdk-dev] [PATCH v8 12/14] eal: add hotplug add/remove functions Jan Blunck
2017-06-30 18:19       ` [dpdk-dev] [PATCH v8 13/14] ethdev: Use embedded rte_device to detach driver Jan Blunck
2017-07-03 23:17         ` Thomas Monjalon
2017-06-30 18:19       ` [dpdk-dev] [PATCH v8 14/14] dev: use new hotplug API in attach Jan Blunck
2017-07-03 22:59       ` [dpdk-dev] [PATCH v8 00/14] bus attach/detach API and hotplug rework 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=20170628091746.GG13355@bidouze.vm.6wind.com \
    --to=gaetan.rivet@6wind.com \
    --cc=dev@dpdk.org \
    --cc=stephen@networkplumber.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).