[libcamera-devel] [PATCH v3] cam: file_sink: Fixes following errors with gcc-13

[Kieran Bingham]

Hi Eric,

Quoting Eric Curtin (2023-02-20 04:55:24)
> ../git/src/cam/file_sink.cpp:92:45: error: possibly dangling reference to a temporary [-Werror=dangling-reference]
>    92 |                 const FrameMetadata::Plane &meta = buffer->metadata().planes()[i];
>       |                                             ^~~~
> ../git/src/cam/file_sink.cpp:92:81: note: the temporary was destroyed at the end of the full expression '(& buffer->libcamera::FrameBuffer::metadata())->libcamera::FrameMetadata::planes().libcamera::Span<const libcamera::FrameMetadata::Plane>::operator[](i)'
>    92 |                 const FrameMetadata::Plane &meta = buffer->metadata().planes()[i];
>       |                                                                                 ^
> cc1plus: all warnings being treated as errors
> 
> Co-developed-by: Khem Raj <raj.khem at gmail.com>
> Signed-off-by: Eric Curtin <ecurtin at redhat.com>

I'm still hoping that holding off a little on this will ensure it's
fixed in the compiler before GCC-13 is released - but I think I can also
already add:

Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

And given a fix was added to gcc-13, but it didn't fix /this/ we may be
getting to the point that we'll simply have to merge this. I guess it
depends on when gcc-13 is actually released?

--
Kieran


> ---
> Changes in v3:
> 
> - Added comment explaining change made because of false negative in gcc
> 
> Changes in v2:
> 
> - Added const
> - Made patch mergeable by accounting for new directory structure
> ---
>  src/apps/cam/file_sink.cpp | 14 +++++++++-----
>  1 file changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/src/apps/cam/file_sink.cpp b/src/apps/cam/file_sink.cpp
> index b32aad24..01846cdb 100644
> --- a/src/apps/cam/file_sink.cpp
> +++ b/src/apps/cam/file_sink.cpp
> @@ -114,13 +114,17 @@ void FileSink::writeBuffer(const Stream *stream, FrameBuffer *buffer,
>         }
>  
>         for (unsigned int i = 0; i < buffer->planes().size(); ++i) {
> -               const FrameMetadata::Plane &meta = buffer->metadata().planes()[i];
> -
> +               /*
> +                * formerly "const FrameMetadata::Plane &" 
> +                * causing false negative (gcc 13):
> +                * "possibly dangling reference to a temporary"
> +                */
> +               const unsigned int bytesused = buffer->metadata().planes()[i].bytesused;
>                 Span<uint8_t> data = image->data(i);
> -               unsigned int length = std::min<unsigned int>(meta.bytesused, data.size());
> +               const unsigned int length = std::min<unsigned int>(bytesused, data.size());
>  
> -               if (meta.bytesused > data.size())
> -                       std::cerr << "payload size " << meta.bytesused
> +               if (bytesused > data.size())
> +                       std::cerr << "payload size " << bytesused
>                                   << " larger than plane size " << data.size()
>                                   << std::endl;
>  
> -- 
> 2.39.1
>