[libcamera-devel] [PATCH 3/4] libcamera: device_enumerator_udev: Avoid double list lookup

[Kieran Bingham]

Hi Laurent,

On 12/09/2019 21:03, Laurent Pinchart wrote:
> DeviceEnumeratorUdev::populateMediaDevice() searches for orphan devices
> in an std::list, and if found removes them using std::list::remove().
> This ends up looking up the entry twice. Replace the remove() call with
> erase() to fix it.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
>  src/libcamera/device_enumerator_udev.cpp | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/src/libcamera/device_enumerator_udev.cpp b/src/libcamera/device_enumerator_udev.cpp
> index 210f5c1f2870..c40770911d3d 100644
> --- a/src/libcamera/device_enumerator_udev.cpp
> +++ b/src/libcamera/device_enumerator_udev.cpp
> @@ -182,7 +182,8 @@ int DeviceEnumeratorUdev::populateMediaDevice(const std::shared_ptr<MediaDevice>
>  				       entity->deviceMinor());
>  
>  		/* Take device from orphan list first, if it is in the list. */
> -		if (std::find(orphans_.begin(), orphans_.end(), devnum) != orphans_.end()) {
> +		auto orphan = std::find(orphans_.begin(), orphans_.end(), devnum);
> +		if (orphan != orphans_.end()) {
>  			std::string deviceNode = lookupDeviceNode(devnum);
>  			if (deviceNode.empty())
>  				return -EINVAL;
> @@ -191,7 +192,7 @@ int DeviceEnumeratorUdev::populateMediaDevice(const std::shared_ptr<MediaDevice>
>  			if (ret)
>  				return ret;
>  
> -			orphans_.remove(devnum);
> +			orphans_.erase(orphan);

Ouch, I had to look up the difference between remove and erase. That's a
subtle but important difference :D And I'm sure no one ever gets bitten
by the fact that .remove() does not 'remove' an entry from a container :-D


Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

>  			continue;
>  		}
>  
> 

-- 
Regards
--
Kieran