I'm not entirely happy with Xi Wang's solution to this mess.

The cast to (unsigned) feels like we're just exploiting
a compiler limitation.
The original idea here was to detect overflow and therefore
avoid hard-coding assumptions about the platform arithmetic.
But allowing compilers to assume that overflow can never happen
means that portable C code can never really detect overflow
without hackish workarounds.

The only practical solution seems to be an arbitrary limit
on the size of a string that we can format.

diff --git a/tar/util.c b/tar/util.c
index 4ed2901d5f5a53ba0f6fa58765471c2333d84fc6..34b62718b9609c975c2cd73b4338359a08a02986 100644 (file)
--- a/tar/util.c
+++ b/tar/util.c
@@ -120,16 +120,12 @@ safe_fprintf(FILE *f, const char *fmt, ...)
                        fmtbuff_length = length+1;
                else if (fmtbuff_length < 8192)
                        fmtbuff_length *= 2;
+               else if (fmtbuff_length < 1000000)
+                       fmtbuff_length += fmtbuff_length / 4;
                else {
-                       int old_length = fmtbuff_length;
-                       /* Convert to unsigned to avoid signed overflow,
-                        * otherwise the check may be optimized away. */
-                       fmtbuff_length += (unsigned)fmtbuff_length / 4;
-                       if (old_length > fmtbuff_length) {
-                               length = old_length;
-                               fmtbuff_heap[length-1] = '\0';
-                               break;
-                       }
+                       length = old_length;
+                       fmtbuff_heap[length-1] = '\0';
+                       break;
                }
                free(fmtbuff_heap);
                fmtbuff_heap = malloc(fmtbuff_length);
@@ -153,8 +149,7 @@ safe_fprintf(FILE *f, const char *fmt, ...)
        if (mbtowc(NULL, NULL, 1) == -1) { /* Reset the shift state. */
                /* NOTE: This case may not happen, but it needs to be compiled
                 * safely without warnings by both gcc on linux and clang. */
-               if (fmtbuff_heap != NULL)
-                       free(fmtbuff_heap);
+               free(fmtbuff_heap);
                return;
        }