Hi Paul,
> I installed the patch I proposed yesterday, along with the
> additional patches attached, which merely change the x* functions to
> check for both kinds of overflow.
These changes give me some stomach-ache. I perfectly understand that
you're making a departure from 20 years of C tradition, to get overflows
detected better. I see three issues:
1) You're basically saying "let's use signed integer types for indices",
and you do that in the quotearg.c change.
1.1) The declaration 'unsigned int i;' served also as an information for
the programmer: "Attention, never assign negative values to this variable!"
Changing that to "int i;" hampers maintainability.
This could be solved through a typedef
typedef int index_t; /* keep always >= 0 */
but such a typedef doesn't solve 1.2).
1.2) Essentially, the only remaining use of 'unsigned int' is for packed
bit arrays and bit masks, and for multi-precision arithmetic. But as C
programmer I do want to have integer variables which are always >= 0,
<= PTRDIFF_MAX, <= SIZE_MAX, _and_ I would like to have them checked
against overflow.
For this purpose, it would be good if GCC had a type, say, __gcc_index_t,
that -fsanitize=undefined will make produce a diagnostic is a value < 0
or > PTRDIFF_MAX is assigned.
2) The type __xalloc_count_type is sometimes signed, sometimes unsigned,
depending on platform (like 'char' and 'wchar_t').
Such types produce extra trouble:
* Some bugs can occur only one kind of platform and cannot be reproduced
even with the best intrumentation (-fsanitize=undefined, valgrind, etc.)
on the other kind of platforms.
* Code which is necessary for one platform produces warnings on the
other platforms. Such as:
wchar_t wc = ...;
if (wc >= 0x0000 && wc < 0x110000)
This produces a warning on platforms where wchar_t is unsigned.
3) The type defined in xalloc-oversized.h has much wider applicability:
#if PTRDIFF_MAX < SIZE_MAX
typedef ptrdiff_t __xalloc_count_type;
#else
typedef size_t __xalloc_count_type;
#endif
It becomes one of the basic C types and should therefore deserve a
name with wider scope. Possibly 'gl_index_t' or such.
Bruno
