From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f54.google.com (mail-wm0-f54.google.com [74.125.82.54]) by dpdk.org (Postfix) with ESMTP id 9EAC52BE1 for ; Wed, 28 Jun 2017 11:17:55 +0200 (CEST) Received: by mail-wm0-f54.google.com with SMTP id 62so47203126wmw.1 for ; Wed, 28 Jun 2017 02:17:55 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:content-transfer-encoding:in-reply-to :user-agent; bh=Mhvjfb7w4EYdJ/V+BLjv2Yswqymv1TAd5k9flD4p6Vs=; b=XnkdXs7LZS3uZ9H+1qPsOEqzrDVSrqH8Npoq6R4sxMNd7+m2Zi/2BFikYl7+L9fU+4 p3k/f8uFR6+8fCH99MtrALIq7gFoD1XnDA0BAqQ13DHfNW/wA5uMrfESeC5b5xS5p37b nu3LUfxZOkGCOOiOkRGqFN952497vfVtDx9ugL+E6kJCBDPv2uWYIuPfwV5qXhUJWvVN Z4CdIjnNh+sD/U8kvCCtyDF+78UNaD1715d8gVe5LAfIRqKt+aL9Hh2AMRBSo0pq/4GW IcF6CsMGwRKpLCs82iN359Nfsj+msehfXvgW5BfqZVNI5lXmuDjlsCQMr++Ui7B4PSbx TOaA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to:user-agent; bh=Mhvjfb7w4EYdJ/V+BLjv2Yswqymv1TAd5k9flD4p6Vs=; b=VQN9j/WpA7Q42DtgWx/Y4WFupHgVBzKIwhjQK5B0QNY4BCk7uuuz3zPrLIUz1SxJD0 5pYu1bLT2wCp58kGTtqS0SfCco1/3fE3ZOaMiCcWAZ4TkeyLHBUMTgcvcZmcVaLCJ9cj ieXb8/xo0cgy67txbO5nREIhXhJRdzW8bvFXe9MSR0DGlFX+Mx4f2/KS/n06xHZ+pL11 ys1iD+Dz3+Ys0LVltfAFjpTeffbCchyuY23le7MbWfvyjKmhi0zMFjrTEklviDlS7xOV zzg9BmrxrL1qcPJVDnITWcix+Sq83QphK1y8AdMkmZWxJUkYyAwjfXxQVcJzU5Rn5Hd0 P4aQ== X-Gm-Message-State: AKS2vOz/XBdkcRwKzY3mNYx0m9PSUIBS+wtEHwmimRZXTGgzy0I9TdKL EYN9PyuaWf6kJzPgYK0= X-Received: by 10.28.191.29 with SMTP id p29mr4223710wmf.60.1498641475260; Wed, 28 Jun 2017 02:17:55 -0700 (PDT) Received: from bidouze.vm.6wind.com (host.78.145.23.62.rev.coltfrance.com. [62.23.145.78]) by smtp.gmail.com with ESMTPSA id v13sm6644567wmd.5.2017.06.28.02.17.54 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 28 Jun 2017 02:17:54 -0700 (PDT) Date: Wed, 28 Jun 2017 11:17:46 +0200 From: =?iso-8859-1?Q?Ga=EBtan?= Rivet To: Stephen Hemminger Cc: dev@dpdk.org Message-ID: <20170628091746.GG13355@bidouze.vm.6wind.com> References: <5fe2e5fb2759d1f6d3f86e5dfae5c3fc177299da.1498577192.git.gaetan.rivet@6wind.com> <20170627163514.2ed2ec6b@xeon-e3> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20170627163514.2ed2ec6b@xeon-e3> User-Agent: Mutt/1.5.23 (2014-03-12) Subject: Re: [dpdk-dev] [PATCH v6 09/11] pci: implement find_device bus operation X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 28 Jun 2017 09:17:55 -0000 On Tue, Jun 27, 2017 at 04:35:14PM -0700, Stephen Hemminger wrote: > On Tue, 27 Jun 2017 18:11:16 +0200 > Gaetan Rivet 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