Paul Eggert wrote:
> it's not true that fprintftime "never punts", if I understand
> what you mean by "punt". fprintftime can return 0 even if it has output
> some bytes, if it discovers an error later in the processing. This is
> true regardless of whether the patches proposed below are installed. And
> it makes sense given the semantics of fprintftime.
Oh, indeed, you're right. I had not seen these 'return 0;' statements:
if (_incr >= maxsize - i) \
{ \
errno = ERANGE; \
return 0; \
} \
and
if (ltm.tm_yday < 0)
{
errno = EOVERFLOW;
return 0;
}
> It'd be one thing if fprintftime followed strftime's lead and returned 0
> on error. But that's a squirrelly API, as strftime returns 0 both on
> error and on success. fprintf's API is cleaner, and fprintftime should
> follow fprintf.
"squirrelly API" — I wholeheartedly agree. Not only the 'size_t' return type
had given me the impression that strftime() cannot fail. Also, the problem
is documented in the man page
<https://www.kernel.org/doc/man-pages/online/pages/man3/strftime.3.html> :
RETURN VALUE
...
Note that the return value 0 does not necessarily indicate an
error. For example, in many locales %p yields an empty string.
An empty format string will likewise yield an empty string.
BUGS
If the output string would exceed max bytes, errno is not set.
This makes it impossible to distinguish this error case from cases
where the format string legitimately produces a zero-length output
string. POSIX.1-2001 does not specify any errno settings for
strftime().
But our GNU programs don't use strftime() (or wcsftime(), as in glibc).
They use nstrftime(), with the GNU extensions such as nanosecond support.
> > Just because the GNU tar code could accommodate fprintftime() returning -1
> > in one place, doesn't mean that it would be a good idea to allow this.
>
> But it is a good idea, for the reasons described above. So I propose the
> attached patches to implement it.
I propose to go even a step further: change also the function nstrftime():
- change the return type from size_t to ptrdiff_t,
- change the failure return value from 0 to -1.
That will require code changes in the callers, to increase reliability
with this new API.
I'll work on a proposed patch in that direction now.
Bruno
