[libcamera-devel] [PATCH] libcamera: camera_sensor: Drop const on the return value of sizes()

[Umang Jain]

Hi,

On 4/6/22 17:54, Laurent Pinchart wrote:
> Hi Umang,
>
> On Wed, Apr 06, 2022 at 05:45:29PM +0530, Umang Jain wrote:
>> On 4/6/22 17:23, Laurent Pinchart via libcamera-devel wrote:
>>> The sizes() function returns a value, not a reference. There's no need
>>> for it to be const.
>>>
>>> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>>> ---
>>>    include/libcamera/internal/camera_sensor.h | 2 +-
>>>    src/libcamera/camera_sensor.cpp            | 2 +-
>>>    2 files changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h
>>> index 7fb4ededb4a4..b9f4d7867854 100644
>>> --- a/include/libcamera/internal/camera_sensor.h
>>> +++ b/include/libcamera/internal/camera_sensor.h
>>> @@ -43,7 +43,7 @@ public:
>>>    	const std::string &id() const { return id_; }
>>>    	const MediaEntity *entity() const { return entity_; }
>>>    	const std::vector<unsigned int> &mbusCodes() const { return mbusCodes_; }
>>> -	const std::vector<Size> sizes(unsigned int mbusCode) const;
>>> +	std::vector<Size> sizes(unsigned int mbusCode) const;
>>
>> Doesn't this enable the caller to modify the vector returned by the
>> function? I guess we don't want that, hence it's returned value has been
>> const in the first place?
> It does, but that's not a problem, because the function returns a vector
> by value, not by reference. The caller gets a copy.


Okay.  Then I guess it upto the caller to decide whether or not, to 
treat the returned copy as const or not.
For e.g in CIO2Device::getSensorFormat()

     const auto sizes = sensor_->sizes(code);

Hence

     Reviewed-by: Umang Jain <umang.jain at ideasonboard.com>

>
>>>    	Size resolution() const;
>>>    	const std::vector<controls::draft::TestPatternModeEnum> &testPatternModes() const
>>>    	{
>>> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp
>>> index 8b4406fe8aed..eaa2da6bad32 100644
>>> --- a/src/libcamera/camera_sensor.cpp
>>> +++ b/src/libcamera/camera_sensor.cpp
>>> @@ -526,7 +526,7 @@ int CameraSensor::discoverAncillaryDevices()
>>>     *
>>>     * \return The supported frame sizes for \a mbusCode sorted in increasing order
>>>     */
>>> -const std::vector<Size> CameraSensor::sizes(unsigned int mbusCode) const
>>> +std::vector<Size> CameraSensor::sizes(unsigned int mbusCode) const
>>>    {
>>>    	std::vector<Size> sizes;
>>>