[v1] treewide: Remove top-level `const` from return types
diff mbox series

Message ID 20250603111434.751188-1-barnabas.pocze@ideasonboard.com
State New
Headers show
Series
  • [v1] treewide: Remove top-level `const` from return types
Related show

Commit Message

Barnabás Pőcze June 3, 2025, 11:14 a.m. UTC
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@ideasonboard.com>
---
 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(-)

Comments

Laurent Pinchart June 3, 2025, 11:41 a.m. UTC | #1
Hi Barnabás,

Thank you for the patch.

On Tue, Jun 03, 2025 at 01:14:34PM +0200, Barnabás Pőcze wrote:
> 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@ideasonboard.com>

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

> ---
>  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;
Barnabás Pőcze June 3, 2025, 12:51 p.m. UTC | #2
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@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.


Regards,
Barnabás Pőcze

> 
> Reviewed-by: Stefan Klug <stefan.klug@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
>>
Kieran Bingham June 5, 2025, 8:59 a.m. UTC | #3
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@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@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
> >>
>

Patch
diff mbox series

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;