| Message ID | 20220406115301.4750-1-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Accepted |
| Commit | 1a9587b8f2de010026784c251179a64105f1fc91 |
| Headers | show |
| Series |
|
| Related | show |
Hi Laurent 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@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? > 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; >
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@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. > > 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; > >
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@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@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; >>>
Hi Laurent, On Wed, Apr 06, 2022 at 02:53:01PM +0300, 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@ideasonboard.com> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> Thanks j > --- > 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; > 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; > > -- > Regards, > > Laurent Pinchart >
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; 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;
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@ideasonboard.com> --- include/libcamera/internal/camera_sensor.h | 2 +- src/libcamera/camera_sensor.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)