[v1] libcamera: camera_manager: Take camera id in `std::string_view`

23084 diff mbox series

Do not force the caller to have an `std::string` object as a
simple string view is sufficient to do the lookup.

Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
---
 include/libcamera/camera_manager.h   | 3 ++-
 src/libcamera/camera_manager.cpp     | 2 +-
 src/py/libcamera/py_camera_manager.h | 2 +-
 3 files changed, 4 insertions(+), 3 deletions(-)

Quoting Barnabás Pőcze (2025-03-31 16:09:00)
> Do not force the caller to have an `std::string` object as a
> simple string view is sufficient to do the lookup.
> 
> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>

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

> ---
>  include/libcamera/camera_manager.h   | 3 ++-
>  src/libcamera/camera_manager.cpp     | 2 +-
>  src/py/libcamera/py_camera_manager.h | 2 +-
>  3 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/include/libcamera/camera_manager.h b/include/libcamera/camera_manager.h
> index b50df7825..27835500f 100644
> --- a/include/libcamera/camera_manager.h
> +++ b/include/libcamera/camera_manager.h
> @@ -9,6 +9,7 @@
>  
>  #include <memory>
>  #include <string>
> +#include <string_view>
>  #include <sys/types.h>
>  #include <vector>
>  
> @@ -31,7 +32,7 @@ public:
>         void stop();
>  
>         std::vector<std::shared_ptr<Camera>> cameras() const;
> -       std::shared_ptr<Camera> get(const std::string &id);
> +       std::shared_ptr<Camera> get(std::string_view id);
>  
>         static const std::string &version() { return version_; }
>  
> diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp
> index b28bd0bd2..e62e7193c 100644
> --- a/src/libcamera/camera_manager.cpp
> +++ b/src/libcamera/camera_manager.cpp
> @@ -381,7 +381,7 @@ std::vector<std::shared_ptr<Camera>> CameraManager::cameras() const
>   *
>   * \return Shared pointer to Camera object or nullptr if camera not found
>   */
> -std::shared_ptr<Camera> CameraManager::get(const std::string &id)
> +std::shared_ptr<Camera> CameraManager::get(std::string_view id)
>  {
>         Private *const d = _d();
>  
> diff --git a/src/py/libcamera/py_camera_manager.h b/src/py/libcamera/py_camera_manager.h
> index 3574db236..af69b915e 100644
> --- a/src/py/libcamera/py_camera_manager.h
> +++ b/src/py/libcamera/py_camera_manager.h
> @@ -20,7 +20,7 @@ public:
>         ~PyCameraManager();
>  
>         pybind11::list cameras();
> -       std::shared_ptr<Camera> get(const std::string &name) { return cameraManager_->get(name); }
> +       std::shared_ptr<Camera> get(std::string_view name) { return cameraManager_->get(name); }
>  
>         static const std::string &version() { return CameraManager::version(); }
>  
> -- 
> 2.49.0
>

On Mon, Mar 31, 2025 at 05:14:03PM +0100, Kieran Bingham wrote:
> Quoting Barnabás Pőcze (2025-03-31 16:09:00)
> > Do not force the caller to have an `std::string` object as a
> > simple string view is sufficient to do the lookup.
> > 
> > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
> 
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

I still dislike how C++ requires the internal implementation of a
function to be taken into account to select the right type of string
argument, but in this case I can't imagine a use case where
CameraManager::get() would be implemented with a need to copy the
string. std::string_view should be a safe future-proof pick.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> > ---
> >  include/libcamera/camera_manager.h   | 3 ++-
> >  src/libcamera/camera_manager.cpp     | 2 +-
> >  src/py/libcamera/py_camera_manager.h | 2 +-
> >  3 files changed, 4 insertions(+), 3 deletions(-)
> > 
> > diff --git a/include/libcamera/camera_manager.h b/include/libcamera/camera_manager.h
> > index b50df7825..27835500f 100644
> > --- a/include/libcamera/camera_manager.h
> > +++ b/include/libcamera/camera_manager.h
> > @@ -9,6 +9,7 @@
> >  
> >  #include <memory>
> >  #include <string>
> > +#include <string_view>
> >  #include <sys/types.h>
> >  #include <vector>
> >  
> > @@ -31,7 +32,7 @@ public:
> >         void stop();
> >  
> >         std::vector<std::shared_ptr<Camera>> cameras() const;
> > -       std::shared_ptr<Camera> get(const std::string &id);
> > +       std::shared_ptr<Camera> get(std::string_view id);
> >  
> >         static const std::string &version() { return version_; }
> >  
> > diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp
> > index b28bd0bd2..e62e7193c 100644
> > --- a/src/libcamera/camera_manager.cpp
> > +++ b/src/libcamera/camera_manager.cpp
> > @@ -381,7 +381,7 @@ std::vector<std::shared_ptr<Camera>> CameraManager::cameras() const
> >   *
> >   * \return Shared pointer to Camera object or nullptr if camera not found
> >   */
> > -std::shared_ptr<Camera> CameraManager::get(const std::string &id)
> > +std::shared_ptr<Camera> CameraManager::get(std::string_view id)
> >  {
> >         Private *const d = _d();
> >  
> > diff --git a/src/py/libcamera/py_camera_manager.h b/src/py/libcamera/py_camera_manager.h
> > index 3574db236..af69b915e 100644
> > --- a/src/py/libcamera/py_camera_manager.h
> > +++ b/src/py/libcamera/py_camera_manager.h
> > @@ -20,7 +20,7 @@ public:
> >         ~PyCameraManager();
> >  
> >         pybind11::list cameras();
> > -       std::shared_ptr<Camera> get(const std::string &name) { return cameraManager_->get(name); }
> > +       std::shared_ptr<Camera> get(std::string_view name) { return cameraManager_->get(name); }
> >  
> >         static const std::string &version() { return CameraManager::version(); }
> >