Re: bug in roundup(3) from <sys/param.h>

[Alejandro Colomar <alx.manpages@gmail.com>]

[-- Attachment #1.1: Type: text/plain, Size: 3521 bytes --]

On 1/16/23 21:46, Alejandro Colomar wrote:
> Hi!
> 
> I was trying to understand what roundup() is (defined in <sys/param,h>).
> 
> It seems to be kind of:
> 
> SYNOPSIS
>         #include <sys/param.h>
> 
>         roundup(x, step);
> 
> DESCRIPTION
>         This  macro  rounds  x to the nearest multiple of step that is not less
>         than x.
> 
> I found that it doesn't work for negative numbers; but that's expected, and it 
> could be documented as such.  However, it doesn't work nicely with unsigned 
> integers either: for values close to zero, where wrap around happens, the result 
> is also bogus.  See my experiments below.
> 
> 
> 
> $ sed -n 92,98p /usr/include/x86_64-linux-gnu/sys/param.h
> #ifdef __GNUC__
> # define roundup(x, y)  (__builtin_constant_p (y) && powerof2 (y)             \
>                           ? (((x) + (y) - 1) & ~((y) - 1))                     \
>                           : ((((x) + ((y) - 1)) / (y)) * (y)))
> #else
> # define roundup(x, y)  ((((x) + ((y) - 1)) / (y)) * (y))
> #endif
> 

I came up with this implementation, which increases complexity quite a lot 
(compared to the one liner), but makes the macro work correctly for all input 
(or that's what my tests showed).  It only has UB for signed input when the 
output would overflow <TYPE>_MAX (but of course, there's no way to avoid that).

Apart from working will all input, signed or unsigned, until the end of the 
range, it also has no problems about double evaluation.

If using GCC extensions is a problem, this could be rewritten a bit less safely 
and more standardese.


#define alx_widthof(t)    (sizeof(t) * CHAR_BIT)

#define alx_is_signed(x)  (((typeof(x)) -1) < 0)

#define alx_stype_max(t)  (((((t) 1 << (alx_widthof(t) - 2)) - 1) << 1) + 1)

#define alx_roundup(x, step)                                                  \
({                                                                            \
	__auto_type  x_    = (x);                                             \
	__auto_type  step_ = (step);                                          \
                                                                               \
	if (alx_is_signed(x_)) {                                              \
		if (x_ < 0) {                                                 \
			x_ = x_ / step_ * step_;                              \
		} else if (x_ - 1 > alx_stype_max(typeof(x_)) - step_) {      \
			x_ = ((x_ - 1) / step_ + 1) * step_;                  \
		} else {                                                      \
			x_ = ((x_ - 1 + step_) / step_) * step_;              \
		}                                                             \
	} else {                                                              \
		if (x_ + step_ < step_) {                                     \
			x_ = ((x_ - 1) / step_ + 1) * step_;                  \
		} else {                                                      \
			x_ = ((x_ - 1 + step_) / step_) * step_;              \
		}                                                             \
	}                                                                     \
                                                                               \
	x_;                                                                   \
})


-- 
<http://www.alejandro-colomar.es/>

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]