Paul Eggert wrote: > Last December, as part of some integer-overflow fixes in GNU Tar, I > changed it to assume that fprintftime returns a negative value on error. > The above-cited code now looks like this: > > > len = add_printf (len, > > (tm ? fprintftime (fp, fmt, tm, 0, > > ts.tv_nsec) > > : fprintf (fp, "????""-??""-?? > > ??:??:??"))); > > where add_printf takes intmax_t arguments, checks for intmax_t overflow > when adding, and assumes a negative count means an error occurred. I > made the GNU Tar change assuming that fprintftime acts like printf with > respect to integer overflow errors.
This code is fine. Fortunately it doesn't matter whether fprintftime or fprintf returns -1 here, because GNU tar uses the return value only to determine how many padding spaces to add later. If the stream is in error state, it does not matter how many padding spaces we output to it; they will all be lost. > If we changed fprintftime to return intmax_t, with -1 meaning there was > an error (possibly intmax_t overflow) and errno being set, this would > fix the bug in the above GNU Tar code. There is no bug in the above GNU Tar code. > This change could be combined > with Collin's proposed patch, and the combination would be better than > Bruno's (a), (b), and (c) alternatives because the result of fprintftime > would be well-documented and reliable. Still, it's easier for callers to assume that the return value is >= 0, than to have to handle -1 as a special case. Bruno
