[libcamera-devel] [PATCH] test: byte-stream-buffer: Initialize data array

[Laurent Pinchart]

Hi Niklas,

Thank you for the patch.

On Sat, May 22, 2021 at 05:21:32PM +0200, Niklas Söderlund wrote:
> Fix compiler warning about variable use before being initialized that
> appears with gcc 11.1.0.
> 
>     test/byte-stream-buffer.cpp:31:63: error: ‘data’ may be used uninitialized [-Werror=maybe-uninitialized]

I've installed gcc 11.1.0 to try and reproduce this (I was testing with
gcc 11.0.1). Turns out I had to disable -Db_sanitize=address to get the
error. Not sure if it occurs with gcc 11.0.1 as it got replaced with
11.1.0 and I don't want to recompile it :-) gcc 10.2.0 doesn't choke,
neither with nor without -Db_sanitize=address.

The full error is:

../../test/byte-stream-buffer.cpp: In member function ‘virtual int ByteStreamBufferTest::run()’:
../../test/byte-stream-buffer.cpp:31:63: error: ‘data’ may be used uninitialized [-Werror=maybe-uninitialized]
   31 |                 ByteStreamBuffer wbuf(data.data(), data.size());
      |                                                               ^
In file included from ../../test/byte-stream-buffer.cpp:8:
/usr/lib/gcc/x86_64-pc-linux-gnu/11.1.0/include/g++-v11/array:176:7: note: by argument 1 of type ‘const std::array<unsigned char, 100>*’ to ‘constexpr std::array<_Tp, _Nm>::size_type std::array<_Tp, _Nm>::size() const [with _Tp = unsigned char; long unsigned int _Nm = 100]’ declared here
  176 |       size() const noexcept { return _Nm; }
      |       ^~~~
../../test/byte-stream-buffer.cpp:23:42: note: ‘data’ declared here
   23 |                 std::array<uint8_t, 100> data;
      |

This seems to be a false positive, the size() function returns _Nm,
which is the second argument to the template and is hardcoded to 100.
The error disappears with the following change:

diff --git a/test/byte-stream-buffer.cpp b/test/byte-stream-buffer.cpp
index d606f146f6ef..312be615da50 100644
--- a/test/byte-stream-buffer.cpp
+++ b/test/byte-stream-buffer.cpp
@@ -28,7 +28,8 @@ protected:
 		/*
 		 * Write mode.
 		 */
-		ByteStreamBuffer wbuf(data.data(), data.size());
+		size_t s = data.size();
+		ByteStreamBuffer wbuf(data.data(), s);

 		if (wbuf.base() != data.data() || wbuf.size() != data.size() ||
 		    wbuf.offset() != 0 || wbuf.overflow()) {

It really looks like a compiler bug.

> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
>  test/byte-stream-buffer.cpp | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/test/byte-stream-buffer.cpp b/test/byte-stream-buffer.cpp
> index d606f146f6ef04ed..c911a863d27122ae 100644
> --- a/test/byte-stream-buffer.cpp
> +++ b/test/byte-stream-buffer.cpp
> @@ -20,7 +20,7 @@ class ByteStreamBufferTest : public Test
>  protected:
>  	int run()
>  	{
> -		std::array<uint8_t, 100> data;
> +		std::array<uint8_t, 100> data = {};

As this is a test case, there's no issue in initializing the array.
Could you however add a comment to explain this, to avoid someone
removing the initialization later by mistake ?

		/*
		 * gcc 11.1.0 incorrectly raises a maybe-uninitialized warning
		 * when calling data.size() below (if the address sanitizer is
		 * disabled). Silence it by initializing the array.
		 */

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

>  		unsigned int i;
>  		uint32_t value;
>  		int ret;

-- 
Regards,

Laurent Pinchart