Re: [Bug-gnulib] linebreak.c proposed patches for size-calculation overflows

[Paul Eggert]

Bruno Haible <address@hidden> writes:

> size_t overflow checking is actually a different facility than
> xmalloc(), therefore I think it deserves its own .h file.

Makes sense, but I suspect it's not limited to size_t.

> > +      if (n <= (size_t)(-1) / (sizeof (size_t) + 2 * MB_LEN_MAX)
> 
> This way is not the right path: the code becomes ununderstandable.
> (In this case, I even doubt the correctness: why MB_LEN_MAX? MB_LEN_MAX
> is 6 in glibc, but iconv() can also apply to encodings, like ISO-2022-JP-1,
> where a single character can lead to 8 bytes or more.)

You can substitute whatever value that you like for MB_LEN_MAX; if the
proper value is 8, it should be written down somewhere, and you can
use that.  The resulting code is still more understandable to me than
the xsum replacement (though admittedly this is a style judgment) and
it's certainly more efficient.

> #ifndef SIZE_MAX
> # define SIZE_MAX ((size_t) -1)
> #endif

xsize.h shouldn't define SIZE_MAX, as various standard headers define
it to a better value (one that can be used in #if), and the better
value will collide with this macro definition.  We could define a
differently-named macro, SIZE_MAXIMUM, say.

> /* Sum of two sizes, with overflow check.  */
> static inline size_t
> xsum (size_t size1, size_t size2)
> {
>   size_t sum = size1 + size2;
>   return (sum >= size1 ? sum : SIZE_MAX);
> }

Shouldn't this be a macro, for the same reason that xtimes is a macro?

> /* Sum of three sizes, with overflow check.  */
> static inline size_t
> xsum3 (size_t size1, size_t size2, size_t size3)

I dunno; this looks a bit overkill to me; also, it's less efficient
than doing it by hand.

> /* Check for overflow.  */
> #define SIZE_OVERFLOW_P(SIZE) \
>   ((SIZE) == SIZE_MAX)

I'd rather use a lower-case name beginning with 'x', for consistency.
Perhaps this:

#define xsize_overflow(size) ((size) == SIZE_MAX)

But to be honest, I still prefer the other way of doing it, at least
for linebreak.c.