[PATCH] libcamera: matrix: Fix compilation error in inverse() function

[Laurent Pinchart]

On Thu, May 22, 2025 at 02:53:57PM +0200, Barnabás Pőcze wrote:
> 2025. 05. 22. 14:42 keltezéssel, Laurent Pinchart írta:
> > Some gcc versions report uninitialized variable usage:
> > 
> > In member function ‘constexpr T& libcamera::Span<T, 4294967295>::operator[](size_type) const [with T = unsigned int]’,
> >      inlined from ‘void libcamera::matrixInvert(Span<const T>, Span<T, 4294967295>, unsigned int, Span<T, 4294967295>, Span<unsigned int>)::MatrixAccessor::swap(unsigned int, unsigned int) [with T = float]’ at ../../src/libcamera/matrix.cpp:194:13,
> >      inlined from ‘bool libcamera::matrixInvert(Span<const T>, Span<T, 4294967295>, unsigned int, Span<T, 4294967295>, Span<unsigned int>) [with T = float]’ at ../../src/libcamera/matrix.cpp:255:14:
> > ../../include/libcamera/base/span.h:362:76: error: ‘row’ may be used uninitialized [-Werror=maybe-uninitialized]
> >    362 |         constexpr reference operator[](size_type idx) const { return data()[idx]; }
> >        |                                                                      ~~~~~~^
> > ../../src/libcamera/matrix.cpp: In function ‘bool libcamera::matrixInvert(Span<const T>, Span<T, 4294967295>, unsigned int, Span<T, 4294967295>, Span<unsigned int>) [with T = float]’:
> > ../../src/libcamera/matrix.cpp:232:30: note: ‘row’ was declared here
> >    232 |                 unsigned int row;
> >        |                              ^~~
> > 
> > This is a false positive. Fix it by initializing the variable when
> > declaring it.
> > 
> > Fixes: 6287ceff5aba ("libcamera: matrix: Add inverse() function")
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > ---
> >   src/libcamera/matrix.cpp | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/src/libcamera/matrix.cpp b/src/libcamera/matrix.cpp
> > index ed22263b58f8..b7c07e896538 100644
> > --- a/src/libcamera/matrix.cpp
> > +++ b/src/libcamera/matrix.cpp
> > @@ -229,7 +229,7 @@ bool matrixInvert(Span<const T> dataIn, Span<T> dataOut, unsigned int dim,
> >   		 * Locate the next pivot. To improve numerical stability, use
> >   		 * the row with the largest value in the pivot's column.
> >   		 */
> > -		unsigned int row;
> > +		unsigned int row = pivot;
> 
> I am wondering if it could be set to `dim`, -1, etc. initially, and the below `if`
> could be modified to check `row >= dim`, etc. That would avoid the value equality
> check, which I think is a good thing.

I've considered something similar. When trying to see if it was really a
false positive, I wondered what would happen when all pivot elements are
0, and then realized it means that the matrix isn't invertible. I've
decided to keep the value check, as I think it carries that meaning
better than the row check.

Is there another reason you think a row check would be better ?

> >   		T maxValue{ 0 };
> >   
> >   		for (unsigned int i = pivot; i < dim; ++i) {
> > 
> > base-commit: efdbe3969841e342c30dfdced38b6ad9ad55dccf

-- 
Regards,

Laurent Pinchart