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

[Laurent Pinchart]

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. I'll review a v2 that uses std::abs(), and ideally addresses
alsc.cpp too.

> >> > 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));

-- 
Regards,

Laurent Pinchart