[libcamera-devel] [PATCH 1/2] libcamera: camera configuration: Remove operator[] from doc

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Wed, Aug 25, 2021 at 06:13:02PM +0200, Jacopo Mondi wrote:
> The documentation suggests to use CameraConfiguration::operator[] to
> access the StreamConfiguration it contains, but as CameraConfiguration
> instances are generated by the Camera class and are returned wrapped in
> a unique_ptr<>. The usage of operator[] would require an awkward syntax such
> as (*config)[i].
> 
> Better to suggest the usage of the CameraConfiguration::at() function
> instead to access the StreamConfigurations.
> 
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  src/libcamera/camera.cpp | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> index c20e05014fb9..c4d576b3985b 100644
> --- a/src/libcamera/camera.cpp
> +++ b/src/libcamera/camera.cpp
> @@ -114,9 +114,9 @@ LOG_DECLARE_CATEGORY(Camera)
>   * The CameraConfiguration holds an ordered list of stream configurations. It
>   * supports iterators and operates as a vector of StreamConfiguration instances.
>   * The stream configurations are inserted by addConfiguration(), and the
> - * operator[](int) returns a reference to the StreamConfiguration based on its
> - * insertion index. Accessing a stream configuration with an invalid index
> - * results in undefined behaviour.
> + * at(unsigned int) function returns a reference to the StreamConfiguration

We could say "at(unsigned int) function or operator[](int)" too. I'm
fine with either.

Another small comment, I'd drop the type and just write at() (and
possibly operator[]()) as that's unambiguous. With or without this,

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

> + * based on its insertion index. Accessing a stream configuration with an
> + * invalid index results in undefined behaviour.
>   *
>   * CameraConfiguration instances are retrieved from the camera with
>   * Camera::generateConfiguration(). Applications may then inspect the

-- 
Regards,

Laurent Pinchart