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

[Laurent Pinchart]

On Thu, Aug 15, 2024 at 10:51:38AM +0100, Kieran Bingham wrote:
> 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?
> 
> 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?

Documentation/coding-style.rst states

  Usage of the C compatibility headers is preferred, except for the
  math.h header. Where math.h defines separate functions for different
  argument types (e.g. abs(int), labs(long int), fabs(double) and
  fabsf(float)) and requires the developer to pick the right function,
  cmath defines overloaded functions (std::abs(int), std::abs(long int),
  std::abs(double) and std::abs(float) to let the compiler select the
  right function. This avoids potential errors such as calling abs(int)
  with a float argument, performing an unwanted implicit integer
  conversion. For this reason, cmath is preferred over math.h.

There's also a set of calls to abs() abd fabs() in
src/ipa/rpi/controller/rpi/alsc.cpp which should be replaced by
std::abs().

Maarten, would you like to submit a new version that addresses both ?

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