Re: [PATCH v3 1/4] iio: prepare iio_format_value to handle higher-precision IIO_VAL_DECIMAL64_* formats

[Rodrigo Alencar]

On 26/10/26 11:51, Victor Chernyakin wrote:
> Prepare iio_format_value for an upcoming patch that adds formats up
> to IIO_VAL_DECIMAL64_YOCTO (a scale of 24 digits) by avoiding
> computing 10^scale when doing so would overflow, and by avoiding
> undefined behavior due to abs(LLONG_MIN) (which couldn't happen
> previously).

...

> +/*
> + * Regular abs(LLONG_MIN) causes signed overflow, which is undefined
> + * behavior. This variant avoids that by performing the computation in an
> + * unsigned type, for which negation is always well-defined.
> + */
> +static inline u64 abs_s64_no_overflow(s64 x)
> +{
> +	return x < 0 ? -(u64)x : (u64)x;
> +}

I think I've seen Andy doing mentions to a needed 'safe' abs() a couple of times.
Don't we have something in the tree already? if not, Why not adding a
'unsigned' abs function to math.h? something like:

/**
 * uabs - return absolute value of an argument as an unsigned type
 * @x: the value.
 *
 * Unlike abs(), the result is well defined when @x is the minimum value of
 * a signed type, since the negation is done in the unsigned domain.
 *
 * Return: an absolute value of @x, as the unsigned counterpart of its type.
 */
#define uabs(x)	__uabs_choose_expr(x, long long,			\
		__uabs_choose_expr(x, long,				\
		__uabs_choose_expr(x, int,				\
		__uabs_choose_expr(x, short,				\
		__uabs_choose_expr(x, char,				\
		__builtin_choose_expr(					\
			__builtin_types_compatible_p(typeof(x), char),	\
			({ signed char __x = (x);			\
			   (unsigned char)(__x < 0 ? -(unsigned char)__x :	\
						     (unsigned char)__x); }),	\
			((void)0)))))))

#define __uabs_choose_expr(x, type, other) __builtin_choose_expr(	\
	__builtin_types_compatible_p(typeof(x),   signed type) ||	\
	__builtin_types_compatible_p(typeof(x), unsigned type),		\
	({ signed type __x = (x);					\
	   (unsigned type)(__x < 0 ? -(unsigned type)__x :		\
				     (unsigned type)__x); }), other)

> +
> +#define GREATEST_POWER_OF_10_THAT_FITS_IN_S64 18
> +

that is readable! being local to this file, maybe define this as
IIO_VAL_INTLOG10_S64_MAX or something to indicate the scope of its usage?

>  static ssize_t __iio_format_value(char *buf, size_t offset, unsigned int type,
>  				  int size, const int *vals)
>  {
> @@ -720,13 +732,19 @@ static ssize_t __iio_format_value(char *buf, size_t offset, unsigned int type,
>  		int scale = type - IIO_VAL_DECIMAL64_BASE;
>  		s64 frac;
>  
> -		tmp2 = div64_s64_rem(iio_val_s64_compose(vals[0], vals[1]),
> -				     int_pow(10, scale), &frac);
> +		if (scale <= GREATEST_POWER_OF_10_THAT_FITS_IN_S64) {
> +			tmp2 = div64_s64_rem(iio_val_s64_compose(vals[0], vals[1]),
> +					     int_pow(10, scale), &frac);
> +		} else {
> +			tmp2 = 0;
> +			frac = iio_val_s64_compose(vals[0], vals[1]);
> +		}
> +
>  		if (tmp2 == 0 && frac < 0)
>  			l += sysfs_emit_at(buf, offset, "-");
>  
> -		l += sysfs_emit_at(buf, offset + l, "%lld.%0*lld", tmp2, scale,
> -				   abs(frac));
> +		l += sysfs_emit_at(buf, offset + l, "%lld.%0*llu", tmp2, scale,
> +				   abs_s64_no_overflow(frac));

...

-- 
Kind regards,

Rodrigo Alencar