[PATCH] ipa/rpi/controller: Use fabs for calculating absolute values

[Milan Zamazal]

Laurent Pinchart <laurent.pinchart at ideasonboard.com> writes:

> On Fri, Aug 16, 2024 at 05:12:28PM +0200, Milan Zamazal wrote:
>> Laurent Pinchart <laurent.pinchart at ideasonboard.com> writes:
>> > On Fri, Aug 16, 2024 at 03:40:45PM +0200, Milan Zamazal wrote:
>
>> >> Kieran Bingham <kieran.bingham at ideasonboard.com> writes:
>> >> 
>> >> > Hi Maarten,
>> >> >
>> >> > Thank you for highlighting this. I'm curious that it hasn't come up in
>> >> > our builds yet! What version of clang are you using?
>> >> 
>> >> It happens to me too, with clang 18.1.6 on Fedora 40.
>> >
>> > I'm very surprised, I tested with clang 18.1.8 locally and didn't see
>> > any error. I would have fixed it already otherwise :-)
>> 
>> Interesting.  I checked in another environment with clang 17.0.6 and it
>> also reports the error.  Can it perhaps depend on particular C/C++
>> header setup/version?  When I ask LSP about `abs' at the given place,
>> it points me to the following from stdlib.h:
>> 
>>   extern int abs (int __x) __THROW __attribute__ ((__const__)) __wur;
>
> Well regardless of where the problem comes from, I think we should fix
> it. 

Definitely.  But it would be interesting to know why it hasn't been
caught by the CI.

> I'll review a v2 that uses std::abs(), and ideally addresses alsc.cpp
> too.

Thank you.

>> >> > Quoting Maarten Lankhorst (2024-08-15 10:37:15)
>> >> >> I was getting the following error in clang:
>> >> >> ../src/ipa/rpi/controller/rpi/awb.cpp:508:6: error: using integer absolute value function 'abs' when argument is of floating point type [-Werror,-Wabsolute-value]
>> >> >>         if (abs(denominator) > eps) {
>> >> >>             ^
>> >> >> ../src/ipa/rpi/controller/rpi/awb.cpp:508:6: note: use function 'std::abs' instead
>> >> >
>> >> > Is there a preference against the recommended std::abs?
>> >> >
>> >> >>         if (abs(denominator) > eps) {
>> >> >>             ^~~
>> >> >>             std::abs
>> >> >> 
>> >> >> Signed-off-by: Maarten Lankhorst <dev at lankhorst.se>
>> >> >> ---
>> >> >>  src/ipa/rpi/controller/rpi/awb.cpp | 2 +-
>> >> >>  1 file changed, 1 insertion(+), 1 deletion(-)
>> >> >> 
>> >> >> diff --git a/src/ipa/rpi/controller/rpi/awb.cpp b/src/ipa/rpi/controller/rpi/awb.cpp
>> >> >> index f45525bc..c5750c84 100644
>> >> >> --- a/src/ipa/rpi/controller/rpi/awb.cpp
>> >> >> +++ b/src/ipa/rpi/controller/rpi/awb.cpp
>> >> >> @@ -505,7 +505,7 @@ static double interpolateQuadatric(ipa::Pwl::Point const &a, ipa::Pwl::Point con
>> >> >>         const double eps = 1e-3;
>> >> >>         ipa::Pwl::Point ca = c - a, ba = b - a;
>> >> >>         double denominator = 2 * (ba.y() * ca.x() - ca.y() * ba.x());
>> >> >> -       if (abs(denominator) > eps) {
>> >> >> +       if (fabs(denominator) > eps) {
>> >> >>                 double numerator = ba.y() * ca.x() * ca.x() - ca.y() * ba.x() * ba.x();
>> >> >>                 double result = numerator / denominator + a.x();
>> >> >>                 return std::max(a.x(), std::min(c.x(), result));