On 10/30/25 05:33, Bruno Haible wrote:
Just like programs
rely on
fprintf (stdout, "%d\n", n);
to never produce an error for integer output, they can also rely on
fprintftime() to never punt (even with the recently added 3 non-Gregorian
calendars). It would be extra code in the programs, adding a fallback
like _("unknown time") if fprintftime() failed.
I'm not following. First, many programs check for fprintf returning -1
on error; it's been a standard part of the fprintf API for ages, and
it's confusing for fprintftime to use a different convention for errors.
Sure, programs that ignore fprintf's return value don't care; but
programs that use the value do care.
If there's really a need for a printf variant that always "succeeds" and
never returns an error value, the variant should have a name that
clearly indicates that it's not following the printf convention.
Second, 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.
It would be extra code in the programs, adding a fallback
like _("unknown time") if fprintftime() failed.
? If fprintftime fails due to an output error, the caller's fallback is
not to output a different string; it's to do something else (e.g., exit
with nonzero status, or do what GNU Tar does).
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.
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. These patches assume the other
fprintftime overflow fixes I installed a few minutes ago.
From 318ff715ddd674ea5123ecd21470094b4a07e2fd Mon Sep 17 00:00:00 2001
From: Paul Eggert <[email protected]>
Date: Thu, 30 Oct 2025 23:01:54 -0600
Subject: [PATCH 1/2] fprintftime: return -1 (not 0) on error
* lib/strftime.c (width_add, __strftime_internal):
Return -1 on error.
(__strftime_internal) [FPRINTFTIME]:
Do not bother to restore errno on success,
as that funny errno-restoring business is needed only
with strftime's squirrely API where 0 is returned on
both success and failure.
---
ChangeLog | 9 +++++++++
lib/strftime.c | 10 +++++-----
2 files changed, 14 insertions(+), 5 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 14f4d37b39..75ad909603 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,14 @@
2025-10-30 Paul Eggert <[email protected]>
+ fprintftime: return -1 (not 0) on error
+ * lib/strftime.c (width_add, __strftime_internal):
+ Return -1 on error.
+ (__strftime_internal) [FPRINTFTIME]:
+ Do not bother to restore errno on success,
+ as that funny errno-restoring business is needed only
+ with strftime's squirrely API where 0 is returned on
+ both success and failure.
+
fprintftime: check for overflow due to padding
* lib/strftime.c (__strftime_internal):
Check for result overflow if the padding is too large,
diff --git a/lib/strftime.c b/lib/strftime.c
index e29cd32666..b1e174bdc9 100644
--- a/lib/strftime.c
+++ b/lib/strftime.c
@@ -249,7 +249,7 @@ typedef ptrdiff_t sbyte_count_t;
if (_incr >= maxsize - i) \
{ \
errno = ERANGE; \
- return 0; \
+ return -FPRINTFTIME; \
} \
if (p) \
{ \
@@ -1894,7 +1894,7 @@ __strftime_internal (STREAM_OR_CHAR_T *s, STRFTIME_ARG (size_t maxsize)
if (ckd_add (&i, i, padding) && FPRINTFTIME)
{
errno = ERANGE;
- return 0;
+ return -FPRINTFTIME;
}
width -= padding;
}
@@ -2057,7 +2057,7 @@ __strftime_internal (STREAM_OR_CHAR_T *s, STRFTIME_ARG (size_t maxsize)
if (ltm.tm_yday < 0)
{
errno = EOVERFLOW;
- return 0;
+ return -FPRINTFTIME;
}
/* Generate string value for T using time_t arithmetic;
@@ -2394,11 +2394,11 @@ __strftime_internal (STREAM_OR_CHAR_T *s, STRFTIME_ARG (size_t maxsize)
}
}
-#if ! FPRINTFTIME
+#if !FPRINTFTIME
if (p && maxsize != 0)
*p = L_('\0');
+ errno = saved_errno;
#endif
- errno = saved_errno;
return i;
}
--
2.51.0
From 20102c69ecffe8ef0cf922f2488585f584b6ff9e Mon Sep 17 00:00:00 2001
From: Paul Eggert <[email protected]>
Date: Thu, 30 Oct 2025 23:50:05 -0600
Subject: [PATCH 2/2] fprintftime: return negative on output error
* lib/strftime.c (FPUTC) [FPRINTFTIME]:
New macro. All uses of fputc changed to FPUTC.
(width_cpy) [FPRINTFTIME]: Do not ignore fwrite failure.
(fwrite_lowcase, fwrite_uppcase) [FPRINTFTIME]:
Remove. All uses replaced with inline code.
---
ChangeLog | 11 ++++++++--
NEWS | 1 +
lib/fprintftime.h | 3 ++-
lib/strftime.c | 55 ++++++++++++++++-------------------------------
4 files changed, 31 insertions(+), 39 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 75ad909603..944a2ec46a 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,8 +1,15 @@
2025-10-30 Paul Eggert <[email protected]>
- fprintftime: return -1 (not 0) on error
+ fprintftime: return negative on output error
+ * lib/strftime.c (FPUTC) [FPRINTFTIME]:
+ New macro. All uses of fputc changed to FPUTC.
+ (width_cpy) [FPRINTFTIME]: Do not ignore fwrite failure.
+ (fwrite_lowcase, fwrite_uppcase) [FPRINTFTIME]:
+ Remove. All uses replaced with inline code.
+
+ fprintftime: return negative (not 0) on error
* lib/strftime.c (width_add, __strftime_internal):
- Return -1 on error.
+ Return negative on error.
(__strftime_internal) [FPRINTFTIME]:
Do not bother to restore errno on success,
as that funny errno-restoring business is needed only
diff --git a/NEWS b/NEWS
index 55dfcd95b0..0983148632 100644
--- a/NEWS
+++ b/NEWS
@@ -79,6 +79,7 @@ User visible incompatible changes
Date Modules Changes
2025-10-30 fprintftime The return value is changed from size_t to off64_t.
+ It returns a negative value on error.
2025-08-05 git-merge-changelog This module is removed. Use the package from
https://git.savannah.gnu.org/git/vc-changelog.git
diff --git a/lib/fprintftime.h b/lib/fprintftime.h
index a340b86117..62bbac6b7b 100644
--- a/lib/fprintftime.h
+++ b/lib/fprintftime.h
@@ -32,7 +32,8 @@ extern "C" {
nstrftime format string, FMT) the time data, *TM, and the ZONE
and NANOSECONDS values.
- Return the number of bytes written to the stream (always >= 0). */
+ Return the number of bytes written to the stream,
+ or a negative number (setting errno) if an error occurred. */
off64_t fprintftime (FILE *fp, char const *fmt, struct tm const *tm,
timezone_t zone, int nanoseconds);
diff --git a/lib/strftime.c b/lib/strftime.c
index b1e174bdc9..a2ac72704c 100644
--- a/lib/strftime.c
+++ b/lib/strftime.c
@@ -216,13 +216,20 @@ typedef ptrdiff_t sbyte_count_t;
#endif
#if FPRINTFTIME
-# define memset_byte(P, Len, Byte) \
+# define FPUTC(Byte, P) \
do \
{ \
- for (byte_count_t _i = Len; 0 < _i; _i--) \
- fputc (Byte, P); \
+ int _r = fputc (Byte, P); \
+ if (_r < 0) \
+ return _r; \
} \
while (false)
+
+# define memset_byte(P, Len, Byte) \
+ do \
+ for (byte_count_t _i = Len; 0 < _i; _i--) \
+ FPUTC (Byte, P); \
+ while (false)
# define memset_space(P, Len) memset_byte (P, Len, ' ')
# define memset_zero(P, Len) memset_byte (P, Len, '0')
#elif defined COMPILE_WIDE
@@ -269,7 +276,7 @@ typedef ptrdiff_t sbyte_count_t;
#define add1(c) width_add1 (width, c)
#if FPRINTFTIME
-# define width_add1(width, c) width_add (width, 1, fputc (c, p))
+# define width_add1(width, c) width_add (width, 1, FPUTC (c, p))
#else
# define width_add1(width, c) width_add (width, 1, *p = c)
#endif
@@ -280,19 +287,15 @@ typedef ptrdiff_t sbyte_count_t;
width_add (width, n, \
do \
{ \
+ CHAR_T const *_s = s; \
if (to_lowcase) \
- fwrite_lowcase (p, (s), _n); \
+ for (byte_count_t _i = 0; _i < _n; _i++) \
+ FPUTC (TOLOWER ((UCHAR_T) _s[_i], loc), p); \
else if (to_uppcase) \
- fwrite_uppcase (p, (s), _n); \
- else \
- { \
- /* Ignore the value of fwrite. The caller can determine whether \
- an error occurred by inspecting ferror (P). All known fwrite \
- implementations set the stream's error indicator when they \
- fail due to ENOMEM etc., even though C11 and POSIX.1-2008 do \
- not require this. */ \
- fwrite (s, _n, 1, p); \
- } \
+ for (byte_count_t _i = 0; _i < _n; _i++) \
+ FPUTC (TOUPPER ((UCHAR_T) _s[_i], loc), p); \
+ else if (fwrite (_s, _n, 1, p) != 1) \
+ return -1; \
} \
while (0) \
)
@@ -374,27 +377,7 @@ typedef ptrdiff_t sbyte_count_t;
# pragma GCC diagnostic ignored "-Wstringop-overflow"
#endif
-#if FPRINTFTIME
-static void
-fwrite_lowcase (FILE *fp, const CHAR_T *src, off64_t len)
-{
- while (len-- > 0)
- {
- fputc (TOLOWER ((UCHAR_T) *src, loc), fp);
- ++src;
- }
-}
-
-static void
-fwrite_uppcase (FILE *fp, const CHAR_T *src, off64_t len)
-{
- while (len-- > 0)
- {
- fputc (TOUPPER ((UCHAR_T) *src, loc), fp);
- ++src;
- }
-}
-#else
+#if !FPRINTFTIME
static CHAR_T *memcpy_lowcase (CHAR_T *dest, const CHAR_T *src,
size_t len LOCALE_PARAM);
--
2.51.0
