On 4/3/21 11:17 PM, Marc Nieper-Wißkirchen wrote:
> Does the comparison make any sense, by the way?
Yes, although it's needed only on unusual (and these days perhaps
theoretical?) platforms where SIZE_MAX < PTRDIFF_MAX.
Ah, okay. I didn't think of this possibility.
I hadn't noticed the issue, as the projects I contribute to (coreutils,
etc.) compile with -Wno-sign-compare because gcc -Wsign-compare has too
many false alarms.
I prefer to avoid casts merely to pacify GCC (as casts are too
error-prone), so I installed the attached. I hope it works for you. (If
not, perhaps you can use -Wno-sign-compare too....)
The fix works for me. Thank you very much! IMO, it's much better than asking to compile with "-Wno-sign-compare' because this can (like type casts) silence other, non-false positive warnings.
Speaking of type casts, I don't think they would have been bad here because they would document exactly what was going on here. By writing
SIZE_MAX < (uintmax_t) nbytes
the otherwise implicit type conversion would have been made explicit and using 'uintmax_t' also documents that it is expected that the width of 'nbytes' can be greater than the width of 'size_t'.
This underscores the fact that the xalloc module should use idx_t
instead of size_t pretty much everywhere. If xrealloc's size arg were of
idx_t we wouldn't need any of this hacking. I realize that replacing
size_t with idx_t is an incompatible change to xalloc's API, but it's
time callers started using signed instead of unsigned byte counts as
that helps avoid and/or catch integer-overflow errors better. I'll add
that to my list of things to do for Gnulib.
The philosophy about idx_t could be worth an entry in the manual.
Marc