[PATCH v1] treewide: Remove top-level `const` from return types
Kieran Bingham
kieran.bingham at ideasonboard.com
Thu Jun 5 10:59:00 CEST 2025
Quoting Barnabás Pőcze (2025-06-03 13:51:33)
> Hi
>
> 2025. 06. 03. 14:36 keltezéssel, Stefan Klug írta:
> > Hi Barnabás,
> >
> > Thank you for the patch.
> >
> > Quoting Barnabás Pőcze (2025-06-03 13:14:34)
> >> Top-level `const` qualifiers are not useful, so avoid them. This is done
> >> either by simply removing the top-level `const`, or making the function
> >> return a reference to const where that is appropriate.
> >>
> >> Signed-off-by: Barnabás Pőcze <barnabas.pocze at ideasonboard.com>
> >
> > Do we need to note somewhere that this breaks the ABI?
>
> I don't know, I can't recall any specific mechanism that was agreed upon.
>
> I believe the itanium abi does not encode the return type of non-template
> functions, and as I would expect abi-compliance-checker reports 100% binary
> compatibility; am I missing something? It is definitely an API change but I
> consider it highly unlikely that anyone's code is affected by this change.
Any proposal is helpful here! But I think we should definitely get the
abi checker in CI too - perhaps based on the above - even potentially
checking both ARM and x86 ?
Perhaps:
Changed-Interface: ABI
Changed-Interface: API
Changed-Interface: ABI+API
Changed-Interface:/Interface-Change:/Interface/Changed:/
or something like that ?
If we formalise on something then it will be easy to highlight in the
release notes when impacting changes are made and reference the commits
too.
I could then imagine that the ABI checker test would use the tag to fail
CI if the tag doesn't match it's results...
--
Kieran
> Regards,
> Barnabás Pőcze
>
> >
> > Reviewed-by: Stefan Klug <stefan.klug at ideasonboard.com>
> >
> > Best regards,
> > Stefan
> >
> >> ---
> >> include/libcamera/base/log.h | 2 +-
> >> include/libcamera/geometry.h | 6 +++---
> >> include/libcamera/internal/matrix.h | 2 +-
> >> include/libcamera/internal/v4l2_subdevice.h | 2 +-
> >> include/libcamera/internal/v4l2_videodevice.h | 2 +-
> >> src/apps/cam/drm.h | 4 ++--
> >> src/ipa/libipa/histogram.h | 2 +-
> >> src/libcamera/geometry.cpp | 6 +++---
> >> src/libcamera/pipeline/ipu3/ipu3.cpp | 2 +-
> >> src/libcamera/pipeline/mali-c55/mali-c55.cpp | 8 ++++----
> >> src/libcamera/v4l2_subdevice.cpp | 2 +-
> >> src/libcamera/v4l2_videodevice.cpp | 2 +-
> >> 12 files changed, 20 insertions(+), 20 deletions(-)
> >>
> >> diff --git a/include/libcamera/base/log.h b/include/libcamera/base/log.h
> >> index 8af74b59d..5a232a650 100644
> >> --- a/include/libcamera/base/log.h
> >> +++ b/include/libcamera/base/log.h
> >> @@ -75,7 +75,7 @@ public:
> >> const LogCategory &category() const { return category_; }
> >> const std::string &fileInfo() const { return fileInfo_; }
> >> const std::string &prefix() const { return prefix_; }
> >> - const std::string msg() const { return msgStream_.str(); }
> >> + std::string msg() const { return msgStream_.str(); }
> >>
> >> private:
> >> LIBCAMERA_DISABLE_COPY_AND_MOVE(LogMessage)
> >> diff --git a/include/libcamera/geometry.h b/include/libcamera/geometry.h
> >> index f322e3d5b..d9378efec 100644
> >> --- a/include/libcamera/geometry.h
> >> +++ b/include/libcamera/geometry.h
> >> @@ -31,7 +31,7 @@ public:
> >> int x;
> >> int y;
> >>
> >> - const std::string toString() const;
> >> + std::string toString() const;
> >>
> >> constexpr Point operator-() const
> >> {
> >> @@ -64,7 +64,7 @@ public:
> >> unsigned int height;
> >>
> >> bool isNull() const { return !width && !height; }
> >> - const std::string toString() const;
> >> + std::string toString() const;
> >>
> >> Size &alignDownTo(unsigned int hAlignment, unsigned int vAlignment)
> >> {
> >> @@ -275,7 +275,7 @@ public:
> >> unsigned int height;
> >>
> >> bool isNull() const { return !width && !height; }
> >> - const std::string toString() const;
> >> + std::string toString() const;
> >>
> >> Point center() const;
> >>
> >> diff --git a/include/libcamera/internal/matrix.h b/include/libcamera/internal/matrix.h
> >> index 47513b995..1842389f2 100644
> >> --- a/include/libcamera/internal/matrix.h
> >> +++ b/include/libcamera/internal/matrix.h
> >> @@ -56,7 +56,7 @@ public:
> >>
> >> ~Matrix() = default;
> >>
> >> - const std::string toString() const
> >> + std::string toString() const
> >> {
> >> std::stringstream out;
> >>
> >> diff --git a/include/libcamera/internal/v4l2_subdevice.h b/include/libcamera/internal/v4l2_subdevice.h
> >> index fa2a4a21e..c1cde1df2 100644
> >> --- a/include/libcamera/internal/v4l2_subdevice.h
> >> +++ b/include/libcamera/internal/v4l2_subdevice.h
> >> @@ -66,7 +66,7 @@ struct V4L2SubdeviceFormat {
> >> Size size;
> >> std::optional<ColorSpace> colorSpace;
> >>
> >> - const std::string toString() const;
> >> + std::string toString() const;
> >> };
> >>
> >> std::ostream &operator<<(std::ostream &out, const V4L2SubdeviceFormat &f);
> >> diff --git a/include/libcamera/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h
> >> index ae6a76cb0..6caafc4dc 100644
> >> --- a/include/libcamera/internal/v4l2_videodevice.h
> >> +++ b/include/libcamera/internal/v4l2_videodevice.h
> >> @@ -178,7 +178,7 @@ public:
> >> std::array<Plane, 3> planes;
> >> unsigned int planesCount = 0;
> >>
> >> - const std::string toString() const;
> >> + std::string toString() const;
> >> };
> >>
> >> std::ostream &operator<<(std::ostream &out, const V4L2DeviceFormat &f);
> >> diff --git a/src/apps/cam/drm.h b/src/apps/cam/drm.h
> >> index 1ba83b6eb..30a916d7e 100644
> >> --- a/src/apps/cam/drm.h
> >> +++ b/src/apps/cam/drm.h
> >> @@ -97,9 +97,9 @@ public:
> >>
> >> bool isImmutable() const { return flags_ & DRM_MODE_PROP_IMMUTABLE; }
> >>
> >> - const std::vector<uint64_t> values() const { return values_; }
> >> + const std::vector<uint64_t> &values() const { return values_; }
> >> const std::map<uint32_t, std::string> &enums() const { return enums_; }
> >> - const std::vector<uint32_t> blobs() const { return blobs_; }
> >> + const std::vector<uint32_t> &blobs() const { return blobs_; }
> >>
> >> private:
> >> Type type_;
> >> diff --git a/src/ipa/libipa/histogram.h b/src/ipa/libipa/histogram.h
> >> index a926002c8..8cf8bb6d1 100644
> >> --- a/src/ipa/libipa/histogram.h
> >> +++ b/src/ipa/libipa/histogram.h
> >> @@ -36,7 +36,7 @@ public:
> >> }
> >>
> >> size_t bins() const { return cumulative_.size() - 1; }
> >> - const Span<const uint64_t> data() const { return cumulative_; }
> >> + Span<const uint64_t> data() const { return cumulative_; }
> >> uint64_t total() const { return cumulative_[cumulative_.size() - 1]; }
> >> uint64_t cumulativeFrequency(double bin) const;
> >> double quantile(double q, uint32_t first = 0, uint32_t last = UINT_MAX) const;
> >> diff --git a/src/libcamera/geometry.cpp b/src/libcamera/geometry.cpp
> >> index 81cc8cd53..de76d0c12 100644
> >> --- a/src/libcamera/geometry.cpp
> >> +++ b/src/libcamera/geometry.cpp
> >> @@ -53,7 +53,7 @@ namespace libcamera {
> >> * \brief Assemble and return a string describing the point
> >> * \return A string describing the point
> >> */
> >> -const std::string Point::toString() const
> >> +std::string Point::toString() const
> >> {
> >> std::stringstream ss;
> >> ss << *this;
> >> @@ -133,7 +133,7 @@ std::ostream &operator<<(std::ostream &out, const Point &p)
> >> * \brief Assemble and return a string describing the size
> >> * \return A string describing the size
> >> */
> >> -const std::string Size::toString() const
> >> +std::string Size::toString() const
> >> {
> >> std::stringstream ss;
> >> ss << *this;
> >> @@ -676,7 +676,7 @@ std::ostream &operator<<(std::ostream &out, const SizeRange &sr)
> >> * \brief Assemble and return a string describing the rectangle
> >> * \return A string describing the Rectangle
> >> */
> >> -const std::string Rectangle::toString() const
> >> +std::string Rectangle::toString() const
> >> {
> >> std::stringstream ss;
> >> ss << *this;
> >> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> >> index e31e3879d..ad20810e6 100644
> >> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> >> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> >> @@ -105,7 +105,7 @@ public:
> >> Status validate() override;
> >>
> >> const StreamConfiguration &cio2Format() const { return cio2Configuration_; }
> >> - const ImgUDevice::PipeConfig imguConfig() const { return pipeConfig_; }
> >> + const ImgUDevice::PipeConfig &imguConfig() const { return pipeConfig_; }
> >>
> >> /* Cache the combinedTransform_ that will be applied to the sensor */
> >> Transform combinedTransform_;
> >> diff --git a/src/libcamera/pipeline/mali-c55/mali-c55.cpp b/src/libcamera/pipeline/mali-c55/mali-c55.cpp
> >> index 4acc091bd..28d5da3c8 100644
> >> --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp
> >> +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp
> >> @@ -101,8 +101,8 @@ public:
> >> int loadIPA();
> >>
> >> /* Deflect these functionalities to either TPG or CameraSensor. */
> >> - const std::vector<Size> sizes(unsigned int mbusCode) const;
> >> - const Size resolution() const;
> >> + std::vector<Size> sizes(unsigned int mbusCode) const;
> >> + Size resolution() const;
> >>
> >> int pixfmtToMbusCode(const PixelFormat &pixFmt) const;
> >> const PixelFormat &bestRawFormat() const;
> >> @@ -195,7 +195,7 @@ void MaliC55CameraData::setSensorControls(const ControlList &sensorControls)
> >> delayedCtrls_->push(sensorControls);
> >> }
> >>
> >> -const std::vector<Size> MaliC55CameraData::sizes(unsigned int mbusCode) const
> >> +std::vector<Size> MaliC55CameraData::sizes(unsigned int mbusCode) const
> >> {
> >> if (sensor_)
> >> return sensor_->sizes(mbusCode);
> >> @@ -218,7 +218,7 @@ const std::vector<Size> MaliC55CameraData::sizes(unsigned int mbusCode) const
> >> return sizes;
> >> }
> >>
> >> -const Size MaliC55CameraData::resolution() const
> >> +Size MaliC55CameraData::resolution() const
> >> {
> >> if (sensor_)
> >> return sensor_->resolution();
> >> diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp
> >> index 33279654d..e9c849ac5 100644
> >> --- a/src/libcamera/v4l2_subdevice.cpp
> >> +++ b/src/libcamera/v4l2_subdevice.cpp
> >> @@ -923,7 +923,7 @@ const MediaBusFormatInfo &MediaBusFormatInfo::info(uint32_t code)
> >> * \brief Assemble and return a string describing the format
> >> * \return A string describing the V4L2SubdeviceFormat
> >> */
> >> -const std::string V4L2SubdeviceFormat::toString() const
> >> +std::string V4L2SubdeviceFormat::toString() const
> >> {
> >> std::stringstream ss;
> >> ss << *this;
> >> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
> >> index d53aa2d3c..58a6704ef 100644
> >> --- a/src/libcamera/v4l2_videodevice.cpp
> >> +++ b/src/libcamera/v4l2_videodevice.cpp
> >> @@ -429,7 +429,7 @@ bool V4L2BufferCache::Entry::operator==(const FrameBuffer &buffer) const
> >> * \brief Assemble and return a string describing the format
> >> * \return A string describing the V4L2DeviceFormat
> >> */
> >> -const std::string V4L2DeviceFormat::toString() const
> >> +std::string V4L2DeviceFormat::toString() const
> >> {
> >> std::stringstream ss;
> >> ss << *this;
> >> --
> >> 2.49.0
> >>
>
More information about the libcamera-devel
mailing list