Re: [Bug-gnulib] proposed vasnprintf patches for address arithmetic and stack overflow

[Paul Eggert]

Bruno Haible <address@hidden> writes:

> The patch I'm committing is different from yours:
>   - I don't want to use an unsigned type where I was using 'int',
>     because that kills the well-known idiom of using "-1" denoting "no value".
>     The use of SIZE_MAX instead makes the code less understandable,
>     because in one place SIZE_MAX denotes overflow and in another place
>     it denotes "no value".

OK, but the code should use ptrdiff_t instead of ssize_t for this purpose.
This should avoid the need for explicit checks against SSIZE_MAX.

One can always subtract two pointers into the same array and get a
valid ptrdiff_t value (that is what ptrdiff_t is for, after all), but
this is not necessarily true for ssize_t.  ssize_t is for system calls
that have to returned signed values that are sizes or error
indications, and where the system can impose arbitrary limits on the
sizes that can be returned (limits that are present mostly for
historical reasons back when everyone used 'int' for this stuff).
ptrdiff_t is for when you don't want to impose those arbitrary limits.
All other things being equal (which is the case here), it's better to
not impose arbitrary limits.

>   - Your uses of xsum and xtimes were incomplete: There were assignments
>     like "allocated = 2 * allocated;" that were not checked in your patch.

The checks were not necessary, because later code (which was already
present) fixed up any problems.

>   - Also "allocated = xsum (allocated + 12, allocated);" is incomplete:
>     it fails to diagnose overflow when allocated == (size_t)-8.

That is not a problem, since nobody can allocate an object of size
(size_t) -8, because that would leave only 7 bytes with which to
implement the rest of your program.

I think I've mentioned this assumption before: namely, that if A is
the size of an existing object, then A+S cannot overflow if S is
"sufficiently small" (S == 8 KiB is safe, for example).  This
assumption is a natural consequence of the flat address space model
that is already mentioned in gnulib/README.  I see that I haven't put
this particular detail into gnulib/README though, so I should write
that down.