bug-gnulib@gnu.org mirror (unofficial)
 help / color / mirror / Atom feed
* lib/time.in.h, ctime has portability problems; with clang and groff
@ 2024-02-03 13:25 Bjarni Ingi Gislason
  2024-02-03 21:43 ` Paul Eggert
  2024-02-05 13:41 ` Bruno Haible
  0 siblings, 2 replies; 38+ messages in thread
From: Bjarni Ingi Gislason @ 2024-02-03 13:25 UTC (permalink / raw)
  To: bug-gnulib

  The gnulib module "ctime" declares "ctime" to be deprecated.

  "ctime" is not used in "groff".

  CC       lib/libgnu_a-openat-proc.o
In file included from ../lib/openat-proc.c:25:
In file included from ./lib/sys/stat.h:51:
./lib/time.h:1094:18: warning: ctime has portability problems -
use gnulib module ctime for portability [-Wuser-defined-warnings]
_GL_WARN_ON_USE (ctime, "ctime can overrun buffers in some cases -
"
                 ^
./lib/time.h:956:1: note: from 'diagnose_if' attribute on 'ctime':
_GL_WARN_ON_USE (ctime, "ctime has portability problems - "
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
./lib/sys/select.h:593:19: note: expanded from macro
'_GL_WARN_ON_USE'
  __attribute__ ((__diagnose_if__ (1, message, "warning")))
                  ^                ~
1 warning generated.



^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: lib/time.in.h, ctime has portability problems; with clang and groff
  2024-02-03 13:25 lib/time.in.h, ctime has portability problems; with clang and groff Bjarni Ingi Gislason
@ 2024-02-03 21:43 ` Paul Eggert
  2024-02-03 21:46   ` Let's remove Gnulib's ctime module Paul Eggert
  2024-02-05 14:02   ` lib/time.in.h, ctime has portability problems; with clang and groff Bruno Haible
  2024-02-05 13:41 ` Bruno Haible
  1 sibling, 2 replies; 38+ messages in thread
From: Paul Eggert @ 2024-02-03 21:43 UTC (permalink / raw)
  To: Bjarni Ingi Gislason; +Cc: bug-gnulib

[-- Attachment #1: Type: text/plain, Size: 54 bytes --]

Thanks for reporting that. I installed the attached.


[-- Attachment #2: 0001-ctime-fix-false-positive.patch --]
[-- Type: text/x-patch, Size: 1752 bytes --]

From 0feb561817782449678a602a7b7cd442da6db585 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Sat, 3 Feb 2024 12:05:17 -0800
Subject: [PATCH] ctime: fix false positive

Problem reported by Bjarni Ingi Gislason in:
https://lists.gnu.org/r/bug-gnulib/2024-02/msg00006.html
* lib/time.in.h (ctime): Do not warn about ctime portability,
as there is a more serious warning about it crashing,
and the two warning directives can cause false alarms.
---
 ChangeLog     | 9 +++++++++
 lib/time.in.h | 6 +-----
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 4711be3dd9..84f91b4fc9 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+2024-02-03  Paul Eggert  <eggert@cs.ucla.edu>
+
+	ctime: fix false positive
+	Problem reported by Bjarni Ingi Gislason in:
+	https://lists.gnu.org/r/bug-gnulib/2024-02/msg00006.html
+	* lib/time.in.h (ctime): Do not warn about ctime portability,
+	as there is a more serious warning about it crashing,
+	and the two warning directives can cause false alarms.
+
 2024-01-31  Bruno Haible  <bruno@clisp.org>
 
 	Implement 3 new properties, added by Unicode 15.1.0.
diff --git a/lib/time.in.h b/lib/time.in.h
index ce28f1af25..df99c8abca 100644
--- a/lib/time.in.h
+++ b/lib/time.in.h
@@ -438,11 +438,7 @@ _GL_CXXALIAS_SYS (ctime, char *, (time_t const *__tp));
 _GL_CXXALIASWARN (ctime);
 #  endif
 # elif defined GNULIB_POSIXCHECK
-#  undef ctime
-#  if HAVE_RAW_DECL_CTIME
-_GL_WARN_ON_USE (ctime, "ctime has portability problems - "
-                 "use gnulib module ctime for portability");
-#  endif
+/* No need to warn about portability, as a more serious warning is below.  */
 # endif
 
 /* Convert *TP to a date and time string.  See
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* Let's remove Gnulib's ctime module
  2024-02-03 21:43 ` Paul Eggert
@ 2024-02-03 21:46   ` Paul Eggert
  2024-02-05  8:16     ` Simon Josefsson via Gnulib discussion list
  2024-02-05 14:37     ` Let's remove Gnulib's ctime module Bruno Haible
  2024-02-05 14:02   ` lib/time.in.h, ctime has portability problems; with clang and groff Bruno Haible
  1 sibling, 2 replies; 38+ messages in thread
From: Paul Eggert @ 2024-02-03 21:46 UTC (permalink / raw)
  To: bug-gnulib; +Cc: Bjarni Ingi Gislason

[-- Attachment #1: Type: text/plain, Size: 177 bytes --]

This recent bug relating to ctime suggests that the ctime module is more 
trouble than it's worth now. I propose that we remove it. Proposed patch 
attached (but not installed).

[-- Attachment #2: 0001-ctime-remove-module.patch --]
[-- Type: text/x-patch, Size: 17062 bytes --]

From 7261860a4ce805fc91ce49a2a9658fd4128ec18d Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Sat, 3 Feb 2024 13:41:06 -0800
Subject: [PROPOSED] ctime: remove module
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The ctime module is a maintenance hassle and ctime is a recipe for
undefined behavior on today's platforms.  I don't know of any
package using it, and at this point any such package should be
fixed to use strftime or sprintf anyway.
* MODULES.html.sh (func_all_modules): Remove ctime.
* NEWS: Mention this.
* lib/ctime.c, m4/ctime.m4, modules/ctime: Remove.
* doc/posix-functions/asctime.texi (asctime):
* doc/posix-functions/asctime_r.texi (asctime_r):
* doc/posix-functions/ctime_r.texi (ctime_r):
Refer to ctime’s buffer overflow doc.
* doc/posix-functions/ctime.texi (ctime): Remove mention of module.
Document why we got into this mess.
* doc/posix-functions/ctime_r.texi (ctime_r): Remove now-incorrect
commentary that I think is about old SunOS ctime_r.
* lib/time.in.h (ctime):
* m4/time_h.m4 (gl_TIME_H_REQUIRE_DEFAULTS, gl_TIME_H_DEFAULTS):
* modules/time-h (Depends-on):
* tests/test-time-h-c++.cc:
Remove everything concerning the ctime module.
---
 ChangeLog                          | 22 +++++++++++
 MODULES.html.sh                    |  1 -
 NEWS                               |  3 ++
 doc/posix-functions/asctime.texi   |  4 +-
 doc/posix-functions/asctime_r.texi |  5 ++-
 doc/posix-functions/ctime.texi     | 36 ++++++++++++------
 doc/posix-functions/ctime_r.texi   | 15 ++------
 lib/ctime.c                        | 59 ------------------------------
 lib/time.in.h                      | 23 ------------
 m4/ctime.m4                        | 14 -------
 m4/time_h.m4                       |  2 -
 modules/ctime                      | 32 ----------------
 modules/time-h                     |  2 -
 tests/test-time-h-c++.cc           |  4 --
 14 files changed, 59 insertions(+), 163 deletions(-)
 delete mode 100644 lib/ctime.c
 delete mode 100644 m4/ctime.m4
 delete mode 100644 modules/ctime

diff --git a/ChangeLog b/ChangeLog
index 84f91b4fc9..663e8a150c 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,27 @@
 2024-02-03  Paul Eggert  <eggert@cs.ucla.edu>
 
+	ctime: remove module
+	The ctime module is a maintenance hassle and ctime is a recipe for
+	undefined behavior on today's platforms.  I don't know of any
+	package using it, and at this point any such package should be
+	fixed to use strftime or sprintf anyway.
+	* MODULES.html.sh (func_all_modules): Remove ctime.
+	* NEWS: Mention this.
+	* lib/ctime.c, m4/ctime.m4, modules/ctime: Remove.
+	* doc/posix-functions/asctime.texi (asctime):
+	* doc/posix-functions/asctime_r.texi (asctime_r):
+	* doc/posix-functions/ctime_r.texi (ctime_r):
+	Refer to ctime’s buffer overflow doc.
+	* doc/posix-functions/ctime.texi (ctime): Remove mention of module.
+	Document why we got into this mess.
+	* doc/posix-functions/ctime_r.texi (ctime_r): Remove now-incorrect
+	commentary that I think is about old SunOS ctime_r.
+	* lib/time.in.h (ctime):
+	* m4/time_h.m4 (gl_TIME_H_REQUIRE_DEFAULTS, gl_TIME_H_DEFAULTS):
+	* modules/time-h (Depends-on):
+	* tests/test-time-h-c++.cc:
+	Remove everything concerning the ctime module.
+
 	ctime: fix false positive
 	Problem reported by Bjarni Ingi Gislason in:
 	https://lists.gnu.org/r/bug-gnulib/2024-02/msg00006.html
diff --git a/MODULES.html.sh b/MODULES.html.sh
index 76a6291303..b42689a9f9 100755
--- a/MODULES.html.sh
+++ b/MODULES.html.sh
@@ -1679,7 +1679,6 @@ func_all_modules ()
 
   func_begin_table
   func_module atexit
-  func_module ctime
   func_module localtime
   func_module strtod
   func_module strerror
diff --git a/NEWS b/NEWS
index a7c22ecb33..fe140c3cb2 100644
--- a/NEWS
+++ b/NEWS
@@ -74,6 +74,9 @@ User visible incompatible changes
 
 Date        Modules         Changes
 
+2024-02-03  ctime           Removed.  Portable code should use strftime
+                            (or even sprintf) instead.
+
 2023-09-03  same-inode      SAME_INODE, ST_BLKSIZE and ST_NBLOCKS args
             stat-size       must be addressable lvalues.
 
diff --git a/doc/posix-functions/asctime.texi b/doc/posix-functions/asctime.texi
index c212a76118..7b19deaa22 100644
--- a/doc/posix-functions/asctime.texi
+++ b/doc/posix-functions/asctime.texi
@@ -18,5 +18,7 @@ Likewise, POSIX says this function is obsolescent and it is planned to be
 removed in a future version.
 Portable applications can use @code{strftime} (or even @code{sprintf}) instead.
 @item
-This function may overflow its internal buffer if an invalid year is passed.
+This function may overflow its internal buffer if its argument
+specifies a time before the year 1000 or after the year 9999.
+@xref{ctime}.
 @end itemize
diff --git a/doc/posix-functions/asctime_r.texi b/doc/posix-functions/asctime_r.texi
index 3b19860363..ae527bd197 100644
--- a/doc/posix-functions/asctime_r.texi
+++ b/doc/posix-functions/asctime_r.texi
@@ -25,6 +25,7 @@ POSIX says this function is obsolescent and it is planned to be
 removed in a future version.
 Use the function @code{strftime} (or even @code{sprintf}) instead.
 @item
-This function may put more than 26 bytes into the argument buffer if an
-invalid year is passed.
+This function may overflow its output buffer if its argument
+specifies a time before the year 1000 or after the year 9999.
+@xref{ctime}.
 @end itemize
diff --git a/doc/posix-functions/ctime.texi b/doc/posix-functions/ctime.texi
index bab929c08c..421f985261 100644
--- a/doc/posix-functions/ctime.texi
+++ b/doc/posix-functions/ctime.texi
@@ -4,15 +4,6 @@
 
 POSIX specification:@* @url{https://pubs.opengroup.org/onlinepubs/9699919799/functions/ctime.html}
 
-Gnulib module: ctime
-
-Portability problems fixed by Gnulib:
-@itemize
-@item
-On native Windows platforms (mingw, MSVC), this function works incorrectly
-when the environment variable @code{TZ} has been set by Cygwin.
-@end itemize
-
 Portability problems not fixed by Gnulib:
 @itemize
 @item
@@ -22,7 +13,27 @@ removed in a future version.
 Portable applications can use @code{localtime_r} and @code{strftime}
 (or even @code{sprintf}) instead.
 @item
-This function may overflow its internal buffer if an invalid year is passed.
+This function may overflow its internal buffer if its argument
+specifies a time before the year 1000 or after the year 9999.
+
+Here is some history about this.
+This function was safe to use decades ago when @code{time_t} was narrow
+and there was no @code{strftime} or internationalization.
+Code could call @code{ctime} and then select the parts needed.
+For example, in Unix 7th Edition @file{/usr/src/cmd/ls.c} (1979):
+
+@example
+cp = ctime(&p->lmtime);
+if(p->lmtime < year)
+        printf(" %-7.7s %-4.4s ", cp+4, cp+20); else
+        printf(" %-12.12s ", cp+4);
+@end example
+
+This has well-defined behavior if @code{time_t} is only 32 bits
+and so was OK for circa 1979 platforms.
+However, today's platforms have a @code{time_t} so wide
+that the year might not be in the range [1000, 9999],
+and ISO C says that in this case the behavior of @code{ctime} is undefined.
 @item
 The @code{ctime} function need not be reentrant, and consequently is
 not required to be thread safe.  Implementations of @code{ctime}
@@ -31,7 +42,10 @@ call @code{ctime} at roughly the same time, you might end up with the
 wrong date in one of the threads, or some undefined string.  There is
 a reentrant interface @code{ctime_r}.
 @item
-Native Windows platforms (mingw, MSVC) support only a subset of time
+On native Windows platforms (mingw, MSVC), this function works incorrectly
+when the environment variable @code{TZ} has been set by Cygwin.
+@item
+Native Windows platforms support only a subset of time
 zones supported by GNU or specified by POSIX@.  @xref{tzset}.
 @end itemize
 
diff --git a/doc/posix-functions/ctime_r.texi b/doc/posix-functions/ctime_r.texi
index 8b5dd136cc..1ecf59bbd4 100644
--- a/doc/posix-functions/ctime_r.texi
+++ b/doc/posix-functions/ctime_r.texi
@@ -26,19 +26,10 @@ removed in a future version.
 Use the functions @code{localtime_r} and @code{strftime}
 (or even @code{sprintf}) instead.
 @item
-This function may put more than 26 bytes into the argument buffer if an
-invalid year is passed.
+This function may overflow its output buffer if its argument
+specifies a time before the year 1000 or after the year 9999.
+@xref{ctime}.
 @end itemize
 
-@code{ctime_r} takes a pre-allocated buffer and length of the buffer,
-and returns @code{NULL} on errors.
-The input buffer should be at least 26 bytes in size.  The output
-string is locale-independent.  However, years can have more than 4
-digits if @code{time_t} is sufficiently wide, so the length of the
-required output buffer is not easy to determine.  Increasing the
-buffer size when @code{ctime_r} returns @code{NULL} is not necessarily
-sufficient.  The @code{NULL} return value could mean some other error
-condition, which will not go away by increasing the buffer size.
-
 A more flexible function is @code{strftime}.  However, note that it is
 locale dependent.
diff --git a/lib/ctime.c b/lib/ctime.c
deleted file mode 100644
index a11adc5c74..0000000000
--- a/lib/ctime.c
+++ /dev/null
@@ -1,59 +0,0 @@
-/* Work around platform bugs in ctime.
-   Copyright (C) 2017-2024 Free Software Foundation, Inc.
-
-   This file is free software: you can redistribute it and/or modify
-   it under the terms of the GNU Lesser General Public License as
-   published by the Free Software Foundation; either version 2.1 of the
-   License, or (at your option) any later version.
-
-   This file is distributed in the hope that it will be useful,
-   but WITHOUT ANY WARRANTY; without even the implied warranty of
-   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-   GNU Lesser General Public License for more details.
-
-   You should have received a copy of the GNU Lesser General Public License
-   along with this program.  If not, see <https://www.gnu.org/licenses/>.  */
-
-#include <config.h>
-
-/* Specification.  */
-#include <time.h>
-
-#include <stdlib.h>
-#include <string.h>
-
-#undef ctime
-
-char *
-rpl_ctime (const time_t *tp)
-{
-#if defined _WIN32 && ! defined __CYGWIN__
-  /* Rectify the value of the environment variable TZ.
-     There are four possible kinds of such values:
-       - Traditional US time zone names, e.g. "PST8PDT".  Syntax: see
-         <https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/tzset>
-       - Time zone names based on geography, that contain one or more
-         slashes, e.g. "Europe/Moscow".
-       - Time zone names based on geography, without slashes, e.g.
-         "Singapore".
-       - Time zone names that contain explicit DST rules.  Syntax: see
-         <https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap08.html#tag_08_03>
-     The Microsoft CRT understands only the first kind.  It produces incorrect
-     results if the value of TZ is of the other kinds.
-     But in a Cygwin environment, /etc/profile.d/tzset.sh sets TZ to a value
-     of the second kind for most geographies, or of the first kind in a few
-     other geographies.  If it is of the second kind, neutralize it.  For the
-     Microsoft CRT, an absent or empty TZ means the time zone that the user
-     has set in the Windows Control Panel.
-     If the value of TZ is of the third or fourth kind -- Cygwin programs
-     understand these syntaxes as well --, it does not matter whether we
-     neutralize it or not, since these values occur only when a Cygwin user
-     has set TZ explicitly; this case is 1. rare and 2. under the user's
-     responsibility.  */
-  const char *tz = getenv ("TZ");
-  if (tz != NULL && strchr (tz, '/') != NULL)
-    _putenv ("TZ=");
-#endif
-
-  return ctime (tp);
-}
diff --git a/lib/time.in.h b/lib/time.in.h
index df99c8abca..875abbe5d7 100644
--- a/lib/time.in.h
+++ b/lib/time.in.h
@@ -418,29 +418,6 @@ _GL_WARN_ON_USE (strptime, "strptime is unportable - "
 #  endif
 # endif
 
-/* Convert *TP to a date and time string.  See
-   <https://pubs.opengroup.org/onlinepubs/9699919799/functions/ctime.html>.  */
-# if @GNULIB_CTIME@
-#  if @REPLACE_CTIME@
-#   if !(defined __cplusplus && defined GNULIB_NAMESPACE)
-#    define ctime rpl_ctime
-#   endif
-#   ifndef __cplusplus
-_GL_ATTRIBUTE_DEPRECATED
-#   endif
-_GL_FUNCDECL_RPL (ctime, char *, (time_t const *__tp)
-                                 _GL_ARG_NONNULL ((1)));
-_GL_CXXALIAS_RPL (ctime, char *, (time_t const *__tp));
-#  else
-_GL_CXXALIAS_SYS (ctime, char *, (time_t const *__tp));
-#  endif
-#  if __GLIBC__ >= 2
-_GL_CXXALIASWARN (ctime);
-#  endif
-# elif defined GNULIB_POSIXCHECK
-/* No need to warn about portability, as a more serious warning is below.  */
-# endif
-
 /* Convert *TP to a date and time string.  See
    <https://pubs.opengroup.org/onlinepubs/9699919799/functions/strftime.html>.  */
 # if @GNULIB_STRFTIME@
diff --git a/m4/ctime.m4 b/m4/ctime.m4
deleted file mode 100644
index b6d70868a8..0000000000
--- a/m4/ctime.m4
+++ /dev/null
@@ -1,14 +0,0 @@
-# ctime.m4 serial 4
-dnl Copyright (C) 2017-2024 Free Software Foundation, Inc.
-dnl This file is free software; the Free Software Foundation
-dnl gives unlimited permission to copy and/or distribute it,
-dnl with or without modifications, as long as this notice is preserved.
-
-AC_DEFUN([gl_FUNC_CTIME],
-[
-  AC_REQUIRE([gl_TIME_H_DEFAULTS])
-  AC_REQUIRE([AC_CANONICAL_HOST])
-  case "$host_os" in
-    mingw* | windows*) REPLACE_CTIME=1 ;;
-  esac
-])
diff --git a/m4/time_h.m4 b/m4/time_h.m4
index 32fade0f40..e1a0fa8be8 100644
--- a/m4/time_h.m4
+++ b/m4/time_h.m4
@@ -134,7 +134,6 @@ AC_DEFUN([gl_TIME_MODULE_INDICATOR],
 AC_DEFUN([gl_TIME_H_REQUIRE_DEFAULTS],
 [
   m4_defun(GL_MODULE_INDICATOR_PREFIX[_TIME_H_MODULE_INDICATOR_DEFAULTS], [
-    gl_MODULE_INDICATOR_INIT_VARIABLE([GNULIB_CTIME])
     gl_MODULE_INDICATOR_INIT_VARIABLE([GNULIB_MKTIME])
     gl_MODULE_INDICATOR_INIT_VARIABLE([GNULIB_LOCALTIME])
     gl_MODULE_INDICATOR_INIT_VARIABLE([GNULIB_NANOSLEEP])
@@ -165,7 +164,6 @@ AC_DEFUN([gl_TIME_H_DEFAULTS],
   HAVE_TIMESPEC_GETRES=1;                AC_SUBST([HAVE_TIMESPEC_GETRES])
   dnl Even GNU libc does not have timezone_t yet.
   HAVE_TIMEZONE_T=0;                     AC_SUBST([HAVE_TIMEZONE_T])
-  REPLACE_CTIME=0;                       AC_SUBST([REPLACE_CTIME])
   REPLACE_GMTIME=0;                      AC_SUBST([REPLACE_GMTIME])
   REPLACE_LOCALTIME=0;                   AC_SUBST([REPLACE_LOCALTIME])
   REPLACE_LOCALTIME_R=0;                 AC_SUBST([REPLACE_LOCALTIME_R])
diff --git a/modules/ctime b/modules/ctime
deleted file mode 100644
index 11de22f2d9..0000000000
--- a/modules/ctime
+++ /dev/null
@@ -1,32 +0,0 @@
-Description:
-ctime() function: convert time to string.
-
-Notice:
-The function 'ctime' is deprecated.
-New code should use 'localtime_r' and 'strftime' (or even 'sprintf') instead.
-
-Files:
-lib/ctime.c
-m4/ctime.m4
-
-Depends-on:
-time-h
-
-configure.ac:
-gl_FUNC_CTIME
-gl_CONDITIONAL([GL_COND_OBJ_CTIME], [test $REPLACE_CTIME = 1])
-gl_TIME_MODULE_INDICATOR([ctime])
-
-Makefile.am:
-if GL_COND_OBJ_CTIME
-lib_SOURCES += ctime.c
-endif
-
-Include:
-<time.h>
-
-License:
-LGPLv2+
-
-Maintainer:
-all
diff --git a/modules/time-h b/modules/time-h
index 3b07845b56..f4aa264e6f 100644
--- a/modules/time-h
+++ b/modules/time-h
@@ -32,7 +32,6 @@ time.h: time.in.h $(top_builddir)/config.status $(CXXDEFS_H) $(ARG_NONNULL_H) $(
 	      -e 's|@''PRAGMA_SYSTEM_HEADER''@|@PRAGMA_SYSTEM_HEADER@|g' \
 	      -e 's|@''PRAGMA_COLUMNS''@|@PRAGMA_COLUMNS@|g' \
 	      -e 's|@''NEXT_TIME_H''@|$(NEXT_TIME_H)|g' \
-	      -e 's/@''GNULIB_CTIME''@/$(GNULIB_CTIME)/g' \
 	      -e 's/@''GNULIB_LOCALTIME''@/$(GNULIB_LOCALTIME)/g' \
 	      -e 's/@''GNULIB_MKTIME''@/$(GNULIB_MKTIME)/g' \
 	      -e 's/@''GNULIB_NANOSLEEP''@/$(GNULIB_NANOSLEEP)/g' \
@@ -53,7 +52,6 @@ time.h: time.in.h $(top_builddir)/config.status $(CXXDEFS_H) $(ARG_NONNULL_H) $(
 	      -e 's|@''HAVE_TIMESPEC_GET''@|$(HAVE_TIMESPEC_GET)|g' \
 	      -e 's|@''HAVE_TIMESPEC_GETRES''@|$(HAVE_TIMESPEC_GETRES)|g' \
 	      -e 's|@''HAVE_TIMEZONE_T''@|$(HAVE_TIMEZONE_T)|g' \
-	      -e 's|@''REPLACE_CTIME''@|$(REPLACE_CTIME)|g' \
 	      -e 's|@''REPLACE_GMTIME''@|$(REPLACE_GMTIME)|g' \
 	      -e 's|@''REPLACE_LOCALTIME''@|$(REPLACE_LOCALTIME)|g' \
 	      -e 's|@''REPLACE_LOCALTIME_R''@|$(REPLACE_LOCALTIME_R)|g' \
diff --git a/tests/test-time-h-c++.cc b/tests/test-time-h-c++.cc
index 2f0b6e9645..d77c8cd829 100644
--- a/tests/test-time-h-c++.cc
+++ b/tests/test-time-h-c++.cc
@@ -66,10 +66,6 @@ SIGNATURE_CHECK (GNULIB_NAMESPACE::strptime, char *,
                  (char const *, char const *, struct tm *));
 #endif
 
-#if GNULIB_TEST_CTIME
-SIGNATURE_CHECK (GNULIB_NAMESPACE::ctime, char *, (time_t const *));
-#endif
-
 #if GNULIB_TEST_STRFTIME
 SIGNATURE_CHECK (GNULIB_NAMESPACE::strftime, size_t,
                  (char *, size_t, const char *, const struct tm *));
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* Re: Let's remove Gnulib's ctime module
  2024-02-03 21:46   ` Let's remove Gnulib's ctime module Paul Eggert
@ 2024-02-05  8:16     ` Simon Josefsson via Gnulib discussion list
  2024-02-05  8:59       ` Paul Eggert
  2024-02-08  9:02       ` obsolescent ctime Bruno Haible
  2024-02-05 14:37     ` Let's remove Gnulib's ctime module Bruno Haible
  1 sibling, 2 replies; 38+ messages in thread
From: Simon Josefsson via Gnulib discussion list @ 2024-02-05  8:16 UTC (permalink / raw)
  To: Paul Eggert; +Cc: bug-gnulib, Bjarni Ingi Gislason

[-- Attachment #1: Type: text/plain, Size: 683 bytes --]

Paul Eggert <eggert@cs.ucla.edu> writes:

> This recent bug relating to ctime suggests that the ctime module is
> more trouble than it's worth now. I propose that we remove
> it. Proposed patch attached (but not installed).

Intresting approach -- I don't mind changing any ctime calls to strftime
in code I come across, however I worry about not noticing these.  I
didn't see anything in your patch that would warn about usage of ctime?
Would it make sense for a gnulib ctime module to NOT replace ctime but
warn that this function should really not be used?  Via header macros,
maybe a stub ctime that calls abort, and maybe a 'make syntax-check'
test.  What do you think?

/Simon

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 255 bytes --]

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: Let's remove Gnulib's ctime module
  2024-02-05  8:16     ` Simon Josefsson via Gnulib discussion list
@ 2024-02-05  8:59       ` Paul Eggert
  2024-02-05  9:48         ` Simon Josefsson via Gnulib discussion list
  2024-02-08  9:02       ` obsolescent ctime Bruno Haible
  1 sibling, 1 reply; 38+ messages in thread
From: Paul Eggert @ 2024-02-05  8:59 UTC (permalink / raw)
  To: Simon Josefsson; +Cc: bug-gnulib, Bjarni Ingi Gislason

On 2024-02-05 00:16, Simon Josefsson wrote:
> didn't see anything in your patch that would warn about usage of ctime?
> Would it make sense for a gnulib ctime module to NOT replace ctime but
> warn that this function should really not be used?

The time-h module does that, so there's no need for the ctime module for 
that.

As I recall, Gnulib's ctime module fixes some time zone issues on 
MS-Windows, something that's quite low priority for a function that user 
code shouldn't be calling anyway due to ctime's undefined behavior when 
the time_t arg is out of range.


^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: Let's remove Gnulib's ctime module
  2024-02-05  8:59       ` Paul Eggert
@ 2024-02-05  9:48         ` Simon Josefsson via Gnulib discussion list
  2024-02-05 10:52           ` Simon Josefsson via Gnulib discussion list
  0 siblings, 1 reply; 38+ messages in thread
From: Simon Josefsson via Gnulib discussion list @ 2024-02-05  9:48 UTC (permalink / raw)
  To: Paul Eggert; +Cc: bug-gnulib, Bjarni Ingi Gislason

[-- Attachment #1: Type: text/plain, Size: 936 bytes --]

mån 2024-02-05 klockan 00:59 -0800 skrev Paul Eggert:
> On 2024-02-05 00:16, Simon Josefsson wrote:
> > didn't see anything in your patch that would warn about usage of
> > ctime?
> > Would it make sense for a gnulib ctime module to NOT replace ctime
> > but
> > warn that this function should really not be used?
> 
> The time-h module does that, so there's no need for the ctime module
> for 
> that.
> 
> As I recall, Gnulib's ctime module fixes some time zone issues on 
> MS-Windows, something that's quite low priority for a function that
> user 
> code shouldn't be calling anyway due to ctime's undefined behavior
> when 
> the time_t arg is out of range.

Okay -- I noticed several ctime() uses in GNU InetUtils (and in
somewhat hibernating GNU Shishi..) and will see if we can fix those. 
People seems to be porting GNU InetUtils to Windows so there may be
interest in having this working.

/Simon


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: Let's remove Gnulib's ctime module
  2024-02-05  9:48         ` Simon Josefsson via Gnulib discussion list
@ 2024-02-05 10:52           ` Simon Josefsson via Gnulib discussion list
  2024-02-06  5:14             ` Paul Eggert
  0 siblings, 1 reply; 38+ messages in thread
From: Simon Josefsson via Gnulib discussion list @ 2024-02-05 10:52 UTC (permalink / raw)
  To: Paul Eggert; +Cc: bug-gnulib

[-- Attachment #1: Type: text/plain, Size: 2099 bytes --]

Here are some examples of ctime usage in GNU InetUtils, starting with
inetd (a single-threaded application):

https://git.savannah.gnu.org/cgit/inetutils.git/tree/src/inetd.c?id=aba8d6528e2577eee7fafab3c418ee5bd94c096b#n1710

This prints day of time of the system.  While we could rewrite that to
use strftime, that would complicate the code to support years < 1000 and
years > 9999 as far as I understand.

Another one is in logger, also a single-threaded application:

https://git.savannah.gnu.org/cgit/inetutils.git/tree/src/logger.c?id=aba8d6528e2577eee7fafab3c418ee5bd94c096b#n301

The code discards day of week and year, but is otherwise okay for years
1000..9999.

Another one is in syslogd:

https://git.savannah.gnu.org/cgit/inetutils.git/tree/src/syslogd.c?id=aba8d6528e2577eee7fafab3c418ee5bd94c096b#n1211
https://git.savannah.gnu.org/cgit/inetutils.git/tree/src/syslogd.c?id=aba8d6528e2577eee7fafab3c418ee5bd94c096b#n1343

There is another use inside libls, which is a bit more interesting:

https://git.savannah.gnu.org/cgit/inetutils.git/tree/libls/print.c?id=aba8d6528e2577eee7fafab3c418ee5bd94c096b#n268

Overall, I'm not sure rewriting these to use sprintf/strftime is a clear
improvement, as the code would be uglier.  Maybe some wrapper will help.
I don't like having code that rely on or trigger undefined behaviour
though.

Could gnulib's ctime replacement call abort() when year<1000 or
year>9999?

Another idea is to have gnulib's ctime augment the C standard to have
ctime not be undefined but to return shorter and longer strings, which I
believe is still consistent with the C standard?

For example it would be permitted to return strings like "Wed Jun 30
21:49:08 623\n" or "Wed Jun 30 21:49:08 11147\n".

Callers will need to make sure they handle string lengths != 26 though,
but that could be documented for the gnulib replacement.

I think this solution make sense for these examples, and is somewhat
more in line with what the original wish may have been: "give me a
english-centric human string representation of this time_t value in a
known fixed format".

/Simon

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 255 bytes --]

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: lib/time.in.h, ctime has portability problems; with clang and groff
  2024-02-03 13:25 lib/time.in.h, ctime has portability problems; with clang and groff Bjarni Ingi Gislason
  2024-02-03 21:43 ` Paul Eggert
@ 2024-02-05 13:41 ` Bruno Haible
  2024-02-06  5:27   ` Paul Eggert
  1 sibling, 1 reply; 38+ messages in thread
From: Bruno Haible @ 2024-02-05 13:41 UTC (permalink / raw)
  To: bug-gnulib; +Cc: Bjarni Ingi Gislason

Bjarni Ingi Gislason wrote:
>   "ctime" is not used in "groff".

Indeed, it was replaced with a call to 'asctime' in
https://git.savannah.gnu.org/gitweb/?p=groff.git;a=commitdiff;h=d7bbfb04ea25a82a8597cdef6ebb391cb78ab47c





^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: lib/time.in.h, ctime has portability problems; with clang and groff
  2024-02-03 21:43 ` Paul Eggert
  2024-02-03 21:46   ` Let's remove Gnulib's ctime module Paul Eggert
@ 2024-02-05 14:02   ` Bruno Haible
  1 sibling, 0 replies; 38+ messages in thread
From: Bruno Haible @ 2024-02-05 14:02 UTC (permalink / raw)
  To: bug-gnulib, Paul Eggert; +Cc: Bjarni Ingi Gislason

Paul Eggert wrote:
> Thanks for reporting that. I installed the attached.

Thanks. So, at the root, it was a restriction w.r.t. _GL_WARN_ON_USE,
that we did not know about. Let me document it.


2024-02-05  Bruno Haible  <bruno@clisp.org>

	snippet/warn-on-use: Add comment.
	* lib/warn-on-use.h: Document a restriction of _GL_WARN_ON_USE.

diff --git a/lib/warn-on-use.h b/lib/warn-on-use.h
index 8f4d40dcbe..701013a07f 100644
--- a/lib/warn-on-use.h
+++ b/lib/warn-on-use.h
@@ -32,6 +32,10 @@
    _GL_WARN_ON_USE_ATTRIBUTE is for functions with 'static' or 'inline'
    linkage.
 
+   _GL_WARN_ON_USE should not be used more than once for a given function
+   in a given compilation unit (because this may generate a warning even
+   if the function is never called).
+
    However, one of the reasons that a function is a portability trap is
    if it has the wrong signature.  Declaring FUNCTION with a different
    signature in C is a compilation error, so this macro must use the





^ permalink raw reply related	[flat|nested] 38+ messages in thread

* Re: Let's remove Gnulib's ctime module
  2024-02-03 21:46   ` Let's remove Gnulib's ctime module Paul Eggert
  2024-02-05  8:16     ` Simon Josefsson via Gnulib discussion list
@ 2024-02-05 14:37     ` Bruno Haible
  2024-02-06  6:59       ` Paul Eggert
  1 sibling, 1 reply; 38+ messages in thread
From: Bruno Haible @ 2024-02-05 14:37 UTC (permalink / raw)
  To: bug-gnulib, Paul Eggert; +Cc: Bjarni Ingi Gislason

Hi Paul,

> This recent bug relating to ctime suggests that the ctime module is more 
> trouble than it's worth now. I propose that we remove it. Proposed patch 
> attached (but not installed).

I disagree that the ctime module is "a maintenance hassle". In my view,
its maintenance takes much less effort than the average.

The reported problem was, at the root, a problem with the _GL_WARN_ON_USE
macro, that could have arisen with any other POSIX function as well.

Since my environment for native Windows is based on Cygwin (since that's
generally more reliable than MSYS2) and the ctime problem with $TZ is a
known one, I want to never encounter it again — even if I'm testing or
debugging a package that happens to use ctime.

Also, according to your doc changes (which I'm mostly committing in your name),
a program that makes sure to use ctime() only for dates between 1900 and
2100 is fine.

> * doc/posix-functions/ctime_r.texi (ctime_r): Remove now-incorrect
> commentary that I think is about old SunOS ctime_r.

That comment was not about SunOS. It was my attempt at understanding
and documenting why ctime_r was still not adequate. I had written:

  The input buffer should be at least 26 bytes in size.  The output
  string is locale-independent.  However, years can have more than 4
  digits if @code{time_t} is sufficiently wide, so the length of the
  required output buffer is not easy to determine.  Increasing the
  buffer size when @code{ctime_r} returns @code{NULL} is not necessarily
  sufficient.  The @code{NULL} return value could mean some other error
  condition, which will not go away by increasing the buffer size.

26 bytes is not enough. But 50 should be enough. Can we give the advice
that it's OK to invoke it with a buffer of size 50?


2024-02-05  Paul Eggert  <eggert@cs.ucla.edu>

	doc: Extend doc of *ctime functions.
	* doc/posix-functions/ctime.texi (ctime): Document why we got into
	this mess.
	* doc/posix-functions/asctime.texi (asctime):
	* doc/posix-functions/asctime_r.texi (asctime_r):
	* doc/posix-functions/ctime_r.texi (ctime_r):
	Refer to ctime’s buffer overflow doc.

diff --git a/doc/posix-functions/asctime.texi b/doc/posix-functions/asctime.texi
index c212a76118..7b19deaa22 100644
--- a/doc/posix-functions/asctime.texi
+++ b/doc/posix-functions/asctime.texi
@@ -18,5 +18,7 @@
 removed in a future version.
 Portable applications can use @code{strftime} (or even @code{sprintf}) instead.
 @item
-This function may overflow its internal buffer if an invalid year is passed.
+This function may overflow its internal buffer if its argument
+specifies a time before the year 1000 or after the year 9999.
+@xref{ctime}.
 @end itemize
diff --git a/doc/posix-functions/asctime_r.texi b/doc/posix-functions/asctime_r.texi
index 3b19860363..ae527bd197 100644
--- a/doc/posix-functions/asctime_r.texi
+++ b/doc/posix-functions/asctime_r.texi
@@ -25,6 +25,7 @@
 removed in a future version.
 Use the function @code{strftime} (or even @code{sprintf}) instead.
 @item
-This function may put more than 26 bytes into the argument buffer if an
-invalid year is passed.
+This function may overflow its output buffer if its argument
+specifies a time before the year 1000 or after the year 9999.
+@xref{ctime}.
 @end itemize
diff --git a/doc/posix-functions/ctime.texi b/doc/posix-functions/ctime.texi
index bab929c08c..76c6e0ffe0 100644
--- a/doc/posix-functions/ctime.texi
+++ b/doc/posix-functions/ctime.texi
@@ -22,7 +22,27 @@
 Portable applications can use @code{localtime_r} and @code{strftime}
 (or even @code{sprintf}) instead.
 @item
-This function may overflow its internal buffer if an invalid year is passed.
+This function may overflow its internal buffer if its argument
+specifies a time before the year 1000 or after the year 9999.
+
+Here is some history about this.
+This function was safe to use decades ago when @code{time_t} was narrow
+and there was no @code{strftime} or internationalization.
+Code could call @code{ctime} and then select the parts needed.
+For example, in Unix 7th Edition @file{/usr/src/cmd/ls.c} (1979):
+
+@example
+cp = ctime(&p->lmtime);
+if(p->lmtime < year)
+        printf(" %-7.7s %-4.4s ", cp+4, cp+20); else
+        printf(" %-12.12s ", cp+4);
+@end example
+
+This has well-defined behavior if @code{time_t} is only 32 bits
+and so was OK for circa 1979 platforms.
+However, today's platforms have a @code{time_t} so wide
+that the year might not be in the range [1000, 9999],
+and ISO C says that in this case the behavior of @code{ctime} is undefined.
 @item
 The @code{ctime} function need not be reentrant, and consequently is
 not required to be thread safe.  Implementations of @code{ctime}
diff --git a/doc/posix-functions/ctime_r.texi b/doc/posix-functions/ctime_r.texi
index 8b5dd136cc..be157cb643 100644
--- a/doc/posix-functions/ctime_r.texi
+++ b/doc/posix-functions/ctime_r.texi
@@ -26,8 +26,9 @@
 Use the functions @code{localtime_r} and @code{strftime}
 (or even @code{sprintf}) instead.
 @item
-This function may put more than 26 bytes into the argument buffer if an
-invalid year is passed.
+This function may overflow its output buffer if its argument
+specifies a time before the year 1000 or after the year 9999.
+@xref{ctime}.
 @end itemize
 
 @code{ctime_r} takes a pre-allocated buffer and length of the buffer,





^ permalink raw reply related	[flat|nested] 38+ messages in thread

* Re: Let's remove Gnulib's ctime module
  2024-02-05 10:52           ` Simon Josefsson via Gnulib discussion list
@ 2024-02-06  5:14             ` Paul Eggert
  2024-02-06  8:04               ` Simon Josefsson via Gnulib discussion list
  0 siblings, 1 reply; 38+ messages in thread
From: Paul Eggert @ 2024-02-06  5:14 UTC (permalink / raw)
  To: Simon Josefsson; +Cc: bug-gnulib

On 2024-02-05 02:52, Simon Josefsson wrote:
> Here are some examples of ctime usage in GNU InetUtils, starting with
> inetd (a single-threaded application):
> 
> https://git.savannah.gnu.org/cgit/inetutils.git/tree/src/inetd.c?id=aba8d6528e2577eee7fafab3c418ee5bd94c096b#n1710
> 
> This prints day of time of the system.  While we could rewrite that to
> use strftime, that would complicate the code to support years < 1000 and
> years > 9999 as far as I understand.

Yes, that code has undefined behavior for years outside the range [1000, 
9999]. If the system clock must be in that year range, the code is OK 
as-is. But since inetutils can be run on systems where the system clock 
can be outside that range, it has undefined behavior when someone messes 
with the system clock - something that I expect any attacker worth its 
salt would want to add to its bag of tricks.


> There is another use inside libls, which is a bit more interesting:
> 
> https://git.savannah.gnu.org/cgit/inetutils.git/tree/libls/print.c?id=aba8d6528e2577eee7fafab3c418ee5bd94c096b#n268

Yes, here ctime is used on a file timestamp, which is more problematic. 
It's easy to give a file a timestamp outside the [1000, 9999] range:

touch -d@9223372036854775807 foo

so that part of inetutils has undefined behavior on such files. I think 
glibc ctime dumps core on typical platforms, but we can't rely on that.


> Overall, I'm not sure rewriting these to use sprintf/strftime is a clear
> improvement, as the code would be uglier.

Hmm, well, part of the problem with ctime is that it's so *tempting* to 
use its simple-but-buggy API. Really, the code needs to be changed.


> Another idea is to have gnulib's ctime augment the C standard to have
> ctime not be undefined but to return shorter and longer strings, which I
> believe is still consistent with the C standard?

I would look askance at any Gnulib implementation of ctime that does 
this sort of thing. The ctime API is so poorly designed that callers 
should use some other API. This is partly why C23 is deprecating ctime. 
Gnulib shouldn't encourage ctime's continued use.

Perhaps we could a new module c_nstrftime, which acts like nstrftime but 
operates in the C locale. That should suffice to replace all uses of 
ctime relatively easily.



^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: lib/time.in.h, ctime has portability problems; with clang and groff
  2024-02-05 13:41 ` Bruno Haible
@ 2024-02-06  5:27   ` Paul Eggert
  0 siblings, 0 replies; 38+ messages in thread
From: Paul Eggert @ 2024-02-06  5:27 UTC (permalink / raw)
  To: Bruno Haible, bug-gnulib; +Cc: Bjarni Ingi Gislason

On 2024-02-05 05:41, Bruno Haible wrote:
> Bjarni Ingi Gislason wrote:
>>    "ctime" is not used in "groff".
> Indeed, it was replaced with a call to 'asctime' in
> https://git.savannah.gnu.org/gitweb/?p=groff.git;a=commitdiff;h=d7bbfb04ea25a82a8597cdef6ebb391cb78ab47c

Unfortunately asctime suffers from many of the undefined-behavior 
problems of ctime. On my platform, current groff dumps core if 
SOURCE_DATE_EPOCH=9223372036854775807 in the environment, and this is 
because groff mistakenly assumes gmtime always succeeds (it doesn't) and 
that asctime always has well-defined behavior if gmtime succeeds (it 
doesn't).

groff should not call asctime, for the same reason it shouldn't call ctime.

asctime is also deprecated in C23, and this is for good reason.


^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: Let's remove Gnulib's ctime module
  2024-02-05 14:37     ` Let's remove Gnulib's ctime module Bruno Haible
@ 2024-02-06  6:59       ` Paul Eggert
  2024-02-06  8:37         ` Bruno Haible
  0 siblings, 1 reply; 38+ messages in thread
From: Paul Eggert @ 2024-02-06  6:59 UTC (permalink / raw)
  To: Bruno Haible, bug-gnulib; +Cc: Bjarni Ingi Gislason

[-- Attachment #1: Type: text/plain, Size: 2444 bytes --]

On 2024-02-05 06:37, Bruno Haible wrote:

> I disagree that the ctime module is "a maintenance hassle". In my view,
> its maintenance takes much less effort than the average.

That depends on what we're averaging over. (Certainly over the past day 
it's been more than average. :-)


> Since my environment for native Windows is based on Cygwin (since that's
> generally more reliable than MSYS2) and the ctime problem with $TZ is a
> known one, I want to never encounter it again — even if I'm testing or
> debugging a package that happens to use ctime.

If you want to keep maintaining that TZ fix then let's keep the ctime 
module. Still, Gnulib should not encourage ctime's use.


> Also, according to your doc changes (which I'm mostly committing in your name),
> a program that makes sure to use ctime() only for dates between 1900 and
> 2100 is fine.

I think the range is [1000, 9999]; not sure where [1900, 2100] came from.


> That comment was not about SunOS. It was my attempt at understanding
> and documenting why ctime_r was still not adequate. I had written:

What you had written was a logical consequence of this earlier part  of 
the Gnulib manual:

> @code{ctime_r} takes a pre-allocated buffer and length of the buffer,
> and returns @code{NULL} on errors.

That was wrong on two counts. First, POSIX ctime_r doesn't have a length 
argument (though traditional SunOS ctime_r does). Second, POSIX ctime_r 
has undefined behavior on errors. Since what you wrote depended on an 
incorrect characterization of ctime_r, there seems little point to 
keeping it; as far as I know the comment is useful only on Solaris when 
_POSIX_PTHREAD_SEMANTICS is not defined, something that is impractical 
if Gnulib's 'extensions' module is used (which it should be).


> 26 bytes is not enough. But 50 should be enough. Can we give the advice
> that it's OK to invoke it with a buffer of size 50?

Heading in that direction would require testing/examination on various 
systems, some of which we lack source code for, and ctime_r is going 
away soon; surely it's not worth the effort.

If Gnulib-using code really needs a ctime substitute we should define 
one with a suitable API (e.g., c_nstrftime).

For now I installed the attached. Please feel free to resurrect the 
SunOS ctime_r commentary if you think it useful (but it should be 
labeled more clearly as being for Solaris).

[-- Attachment #2: 0001-ctime-improve-doc.patch --]
[-- Type: text/x-patch, Size: 7609 bytes --]

From 92a981f18fb8eea357336bcd6638b37ac36aaa4d Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Mon, 5 Feb 2024 22:48:29 -0800
Subject: [PATCH] ctime: improve doc

* doc/posix-functions/asctime.texi (asctime):
* doc/posix-functions/asctime_r.texi (asctime_r):
* doc/posix-functions/ctime_r.texi (ctime_r):
* doc/posix-functions/ctime.texi (ctime):
Mention locale problem of strftime more consistently.  Improve
wording.  For ctime and ctime_r, mention that localtime_r can
fail.
* doc/posix-functions/ctime.texi (ctime): Move history section
to end and spiff up a bit.
* doc/posix-functions/ctime_r.texi (ctime_r): Omit commentary that
assumes traditional SunOS ctime_r API; it was confusing and not
useful for Gnulib apps, which assume the POSIX API.
---
 ChangeLog                          | 16 ++++++++++
 doc/posix-functions/asctime.texi   |  3 +-
 doc/posix-functions/asctime_r.texi |  3 +-
 doc/posix-functions/ctime.texi     | 50 +++++++++++++++++-------------
 doc/posix-functions/ctime_r.texi   | 14 +--------
 5 files changed, 50 insertions(+), 36 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 2e774f6114..4813e18c7b 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,19 @@
+2024-02-03  Paul Eggert  <eggert@cs.ucla.edu>
+
+	ctime: improve doc
+	* doc/posix-functions/asctime.texi (asctime):
+	* doc/posix-functions/asctime_r.texi (asctime_r):
+	* doc/posix-functions/ctime_r.texi (ctime_r):
+	* doc/posix-functions/ctime.texi (ctime):
+	Mention locale problem of strftime more consistently.  Improve
+	wording.  For ctime and ctime_r, mention that localtime_r can
+	fail.
+	* doc/posix-functions/ctime.texi (ctime): Move history section
+	to end and spiff up a bit.
+	* doc/posix-functions/ctime_r.texi (ctime_r): Omit commentary that
+	assumes traditional SunOS ctime_r API; it was confusing and not
+	useful for Gnulib apps, which assume the POSIX API.
+
 2024-02-05  Bruno Haible  <bruno@clisp.org>
 
 	Further improve cross-compilation for midipix.
diff --git a/doc/posix-functions/asctime.texi b/doc/posix-functions/asctime.texi
index 7b19deaa22..13aae8385a 100644
--- a/doc/posix-functions/asctime.texi
+++ b/doc/posix-functions/asctime.texi
@@ -17,8 +17,9 @@ This function is deprecated in C23.
 Likewise, POSIX says this function is obsolescent and it is planned to be
 removed in a future version.
 Portable applications can use @code{strftime} (or even @code{sprintf}) instead.
+However, @code{strftime} is locale dependent.
 @item
 This function may overflow its internal buffer if its argument
-specifies a time before the year 1000 or after the year 9999.
+specifies a year before 1000 or after 9999.
 @xref{ctime}.
 @end itemize
diff --git a/doc/posix-functions/asctime_r.texi b/doc/posix-functions/asctime_r.texi
index ae527bd197..e948d3c47b 100644
--- a/doc/posix-functions/asctime_r.texi
+++ b/doc/posix-functions/asctime_r.texi
@@ -24,8 +24,9 @@ mingw, MSVC 14.
 POSIX says this function is obsolescent and it is planned to be
 removed in a future version.
 Use the function @code{strftime} (or even @code{sprintf}) instead.
+However, @code{strftime} is locale dependent.
 @item
 This function may overflow its output buffer if its argument
-specifies a time before the year 1000 or after the year 9999.
+specifies a year before 1000 or after 9999.
 @xref{ctime}.
 @end itemize
diff --git a/doc/posix-functions/ctime.texi b/doc/posix-functions/ctime.texi
index 76c6e0ffe0..3a1aa489f4 100644
--- a/doc/posix-functions/ctime.texi
+++ b/doc/posix-functions/ctime.texi
@@ -21,28 +21,10 @@ Likewise, POSIX says this function is obsolescent and it is planned to be
 removed in a future version.
 Portable applications can use @code{localtime_r} and @code{strftime}
 (or even @code{sprintf}) instead.
+However, @code{localtime_r} can fail and @code{strftime} is locale dependent.
 @item
 This function may overflow its internal buffer if its argument
 specifies a time before the year 1000 or after the year 9999.
-
-Here is some history about this.
-This function was safe to use decades ago when @code{time_t} was narrow
-and there was no @code{strftime} or internationalization.
-Code could call @code{ctime} and then select the parts needed.
-For example, in Unix 7th Edition @file{/usr/src/cmd/ls.c} (1979):
-
-@example
-cp = ctime(&p->lmtime);
-if(p->lmtime < year)
-        printf(" %-7.7s %-4.4s ", cp+4, cp+20); else
-        printf(" %-12.12s ", cp+4);
-@end example
-
-This has well-defined behavior if @code{time_t} is only 32 bits
-and so was OK for circa 1979 platforms.
-However, today's platforms have a @code{time_t} so wide
-that the year might not be in the range [1000, 9999],
-and ISO C says that in this case the behavior of @code{ctime} is undefined.
 @item
 The @code{ctime} function need not be reentrant, and consequently is
 not required to be thread safe.  Implementations of @code{ctime}
@@ -55,5 +37,31 @@ Native Windows platforms (mingw, MSVC) support only a subset of time
 zones supported by GNU or specified by POSIX@.  @xref{tzset}.
 @end itemize
 
-A more flexible function is @code{strftime}.  However, note that it is
-locale dependent.
+Although @code{ctime} and related functions @code{asctime}, @code{asctime_r}
+and @code{ctime_r} formerly were plausible to use,
+they are now unsafe in general, and should be avoided.
+
+Decades ago when @code{time_t} was narrow
+and there was no @code{strftime} or internationalization,
+code could call these functions and then select the parts needed.
+For example, in Unix 7th Edition @file{/usr/src/cmd/ls.c} (1979):
+
+@example
+cp = ctime(&p->lmtime);
+if(p->lmtime < year)
+        printf(" %-7.7s %-4.4s ", cp+4, cp+20); else
+        printf(" %-12.12s ", cp+4);
+@end example
+
+@noindent
+This had well-defined behavior when @code{time_t} was only 32 bits
+and so was OK for circa 1979 platforms.
+
+However, today's platforms have a @code{time_t} so wide
+that the year might not be in the range [1000, 9999].
+In this case the behavior of @code{ctime} is undefined
+and some platforms behave badly, overrunning a buffer;
+and even on platforms where no buffer overrun occurs,
+the 7th Edition code can generate wrong output for out-of-range years,
+because it incorrectly assumes that every year is represented by
+exactly four digits.
diff --git a/doc/posix-functions/ctime_r.texi b/doc/posix-functions/ctime_r.texi
index be157cb643..8b3fcb4ea7 100644
--- a/doc/posix-functions/ctime_r.texi
+++ b/doc/posix-functions/ctime_r.texi
@@ -25,21 +25,9 @@ POSIX says this function is obsolescent and it is planned to be
 removed in a future version.
 Use the functions @code{localtime_r} and @code{strftime}
 (or even @code{sprintf}) instead.
+However, @code{localtime_r} can fail and @code{strftime} is locale dependent.
 @item
 This function may overflow its output buffer if its argument
 specifies a time before the year 1000 or after the year 9999.
 @xref{ctime}.
 @end itemize
-
-@code{ctime_r} takes a pre-allocated buffer and length of the buffer,
-and returns @code{NULL} on errors.
-The input buffer should be at least 26 bytes in size.  The output
-string is locale-independent.  However, years can have more than 4
-digits if @code{time_t} is sufficiently wide, so the length of the
-required output buffer is not easy to determine.  Increasing the
-buffer size when @code{ctime_r} returns @code{NULL} is not necessarily
-sufficient.  The @code{NULL} return value could mean some other error
-condition, which will not go away by increasing the buffer size.
-
-A more flexible function is @code{strftime}.  However, note that it is
-locale dependent.
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* Re: Let's remove Gnulib's ctime module
  2024-02-06  5:14             ` Paul Eggert
@ 2024-02-06  8:04               ` Simon Josefsson via Gnulib discussion list
  2024-02-08 17:00                 ` Bruno Haible
  0 siblings, 1 reply; 38+ messages in thread
From: Simon Josefsson via Gnulib discussion list @ 2024-02-06  8:04 UTC (permalink / raw)
  To: Paul Eggert; +Cc: bug-gnulib

[-- Attachment #1: Type: text/plain, Size: 3385 bytes --]

You convinced me inetutils (and many other programs) has real bugs
related to ctime today that should be fixed.  Now I want to figure out
what the best fix to existing code is.

Paul Eggert <eggert@cs.ucla.edu> writes:

>> Another idea is to have gnulib's ctime augment the C standard to have
>> ctime not be undefined but to return shorter and longer strings, which I
>> believe is still consistent with the C standard?
>
> I would look askance at any Gnulib implementation of ctime that does
> this sort of thing. The ctime API is so poorly designed that callers 
> should use some other API. This is partly why C23 is deprecating
> ctime. Gnulib shouldn't encourage ctime's continued use.

If gnulib provides a simple to use replacement with clear documented
semantics and interface, and a clear upgrade path from current ctime, it
seems okay to give up on trying to use the ctime API.

Perhaps more than one upgrade path is needed, to accomodate different
situations: the inetutils examples illustrate some different needs.

If we do a good job here, it may serve as a template solution for this
problem elsewhere.  I see some systems migrate 32-bit time_t to 64-bit
time_t, and if not done carefully, that may introduce reliance on
undefined behaviour for years <1000 and >9999 when calling ctime that
wasn't there before.

> Perhaps we could a new module c_nstrftime, which acts like nstrftime
> but operates in the C locale. That should suffice to replace all uses
> of ctime relatively easily.

Yes, although I would prefer a wrapper to hide the complex strftime
format string needed.

How about the API below?

I'm not confident about the timezone handling: maybe it should set the
tzset variables?  And maybe a c_nctime_r would be useful to provide the
timezone TZ to use?  I'm also not certain about year 0 handling.

/* Convert TIMEP representing the number of seconds elapsed since epoch,
1970-01-01 00:00:00 +0000 (UTC), to a fixed locale-independent string
such as "Wed Jun 30 21:49:08 1993\n" using abbreviations for the days of
the week as "Sun", "Mon", "Tue", "Wed", "Thu", "Fri", and "Sat" and
abbreviations for the months as "Jan", "Feb", "Mar", "Apr", "May",
"Jun", "Jul", "Aug", "Sep", "Oct", "Nov", and "Dec".  The function does
not set the external variables tzname, timezone or daylight, see
tzset(3).  The output is copied into STR which must have room for at
least LEN bytes.  For years 1000 to 9999 inclusive the needed length
will be 26 characters including the final NUL byte, but the required
length may be shorter for years < 1000 and larger for years > 9999.  The
years are not padded with whitespace or zeros, so valid outputs include
strings such as "Wed Jun 30 21:49:08 623\n" for years <1000 and for
years >9999 strings such as "Wed Jun 30 21:49:08 11147\n" and for
negative years strings such as "Wed Jun 30 21:49:08 -42\n".  The
preloptic Gregorian calendar is used for all years, to cover years
before the Gregorian calendar was adopted; and for years before 1 the
ISO 8601 approach to have years 2, 1, 0, -1, and so on is used instead
of having 2 BC, 1 BC, AD 1, AD 2.  If TIMEP cannot be converted into a
string of size LEN, NULL is returned and errno is set to an error,
otherwise on success STR is returned. */

char *c_ctime_r (time_t timep, char *str, size_t len);

/Simon

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 255 bytes --]

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: Let's remove Gnulib's ctime module
  2024-02-06  6:59       ` Paul Eggert
@ 2024-02-06  8:37         ` Bruno Haible
  0 siblings, 0 replies; 38+ messages in thread
From: Bruno Haible @ 2024-02-06  8:37 UTC (permalink / raw)
  To: bug-gnulib, Paul Eggert; +Cc: Bjarni Ingi Gislason

Paul Eggert wrote:
> If you want to keep maintaining that TZ fix then let's keep the ctime 
> module. Still, Gnulib should not encourage ctime's use.

I agree on this.

> > Also, according to your doc changes (which I'm mostly committing in your name),
> > a program that makes sure to use ctime() only for dates between 1900 and
> > 2100 is fine.
> 
> I think the range is [1000, 9999]; not sure where [1900, 2100] came from.

I narrowed the interval, thinking of practical use-cases. Dates before 1582
are unlikely to occur anyway in the C locale, since the Gregorian calendar
began only in that year.

> 
> > That comment was not about SunOS. It was my attempt at understanding
> > and documenting why ctime_r was still not adequate. I had written:
> ...
> That was wrong on two counts. First, POSIX ctime_r doesn't have a length 
> argument (though traditional SunOS ctime_r does).

Oh, you are right. I had missed that there are two different APIs for ctime_r:
the older Solaris one [1] and the POSIX one [2].

> > 26 bytes is not enough. But 50 should be enough. Can we give the advice
> > that it's OK to invoke it with a buffer of size 50?
> 
> Heading in that direction would require testing/examination on various 
> systems, some of which we lack source code for, and ctime_r is going 
> away soon; surely it's not worth the effort.

I see, and I agree.

> For now I installed the attached.

Thanks; looks good.

Bruno

[1] https://docs.oracle.com/cd/E36784_01/html/E36874/ctime-r-3c.html
[2] https://pubs.opengroup.org/onlinepubs/9699919799/functions/ctime.html





^ permalink raw reply	[flat|nested] 38+ messages in thread

* obsolescent ctime
  2024-02-05  8:16     ` Simon Josefsson via Gnulib discussion list
  2024-02-05  8:59       ` Paul Eggert
@ 2024-02-08  9:02       ` Bruno Haible
  2024-02-09  2:20         ` Paul Eggert
  1 sibling, 1 reply; 38+ messages in thread
From: Bruno Haible @ 2024-02-08  9:02 UTC (permalink / raw)
  To: bug-gnulib, Simon Josefsson; +Cc: Paul Eggert

Simon Josefsson wrote:
> I don't mind changing any ctime calls to strftime
> in code I come across, however I worry about not noticing these.  I
> didn't see anything in your patch that would warn about usage of ctime?

A regular expression search can find such uses as well.

All of ctime, ctime_r, asctime, asctime_r are affected in the same way.[1][2]

Using https://codesearch.debian.net with the search string
  \b(as)?ctime(_r)?\b *[(] filetype:c
(in regex mode) lists occurrences in more than 1300 packages (!).

For example, in specific GNU packages:
https://codesearch.debian.net/search?q=%5Cb%28as%29%3Fctime%28_r%29%3F%5Cb+*%5B%28%5D+filetype%3Ac+package%3Abinutils&literal=0
https://codesearch.debian.net/search?q=%5Cb%28as%29%3Fctime%28_r%29%3F%5Cb+*%5B%28%5D+filetype%3Ac+package%3Acpio&literal=0
https://codesearch.debian.net/search?q=%5Cb%28as%29%3Fctime%28_r%29%3F%5Cb+*%5B%28%5D+filetype%3Ac+package%3Aemacs&literal=0
https://codesearch.debian.net/search?q=%5Cb%28as%29%3Fctime%28_r%29%3F%5Cb+*%5B%28%5D+filetype%3Ac+package%3Afindutils&literal=0
https://codesearch.debian.net/search?q=%5Cb%28as%29%3Fctime%28_r%29%3F%5Cb+*%5B%28%5D+filetype%3Ac+package%3Agawk&literal=0
https://codesearch.debian.net/search?q=%5Cb%28as%29%3Fctime%28_r%29%3F%5Cb+*%5B%28%5D+filetype%3Acpp+package%3Agroff&literal=0
https://codesearch.debian.net/search?q=%5Cb%28as%29%3Fctime%28_r%29%3F%5Cb+*%5B%28%5D+filetype%3Ac+package%3Ainetutils&literal=0

Bruno

[1] https://pubs.opengroup.org/onlinepubs/9699919799/functions/ctime.html
[2] https://pubs.opengroup.org/onlinepubs/9699919799/functions/asctime.html





^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: Let's remove Gnulib's ctime module
  2024-02-06  8:04               ` Simon Josefsson via Gnulib discussion list
@ 2024-02-08 17:00                 ` Bruno Haible
  2024-02-08 18:22                   ` Paul Eggert
  0 siblings, 1 reply; 38+ messages in thread
From: Bruno Haible @ 2024-02-08 17:00 UTC (permalink / raw)
  To: bug-gnulib, Simon Josefsson; +Cc: Paul Eggert

Simon Josefsson wrote:
> Yes, although I would prefer a wrapper to hide the complex strftime
> format string needed.
> 
> How about the API below?
> ...
> char *c_ctime_r (time_t timep, char *str, size_t len);

I think more arguments are needed.

* Hiding the "complex" format string is not the right approach.
  Rather, this format string is what makes the function flexible
  enough.
  What one can do, in order to help users with this format string, is:
    - a list of format string directives, like the one I added to
      strftime.h,
    - maybe some constants that expand to format strings, e.g.
        #define TIME_FORMAT_ISO_8601 "%Y-%m-%d %H:%M:%S"

* Many variants of such a simple function are possible:
    - (a) time_t vs. struct tm const * argument,
    - (b) interpret the arguments as UTC (from gmtime) vs.
      as local time (from localtime),
    - (c) uses nanoseconds, microseconds, milliseconds, or only seconds,
    - (d) obeys LC_TIME locale category vs. en_US / C locale,
    - (e) return value: uses preallocated buffer (for speed) vs. uses malloc.
  But we don't want to define 2 * 2 * 4 * 2 * 2 = 64 "simple" functions.

  I would propose 2 new functions, like this:
    - (a) one function taking a time_t, one function taking a struct tm const *.
    - (b) accept a 'bool' argument. A timezone_t argument is overkill, since
          most of the uses are either UTC or local time.
    - (c) accept an 'int ns' argument. Passing 0 if not needed is easy.
    - (d) accept a 'bool' argument.
    - (e) use the return value convention from GNU libunistring. It is
          designed to be easily specializable to "preallocated buffer" or
          "use malloc":
          "Functions returning a string result take a ‘(RESULTBUF, LENGTHP)’
           argument pair.  If RESULTBUF is not NULL and the result fits into
           ‘*LENGTHP’ units, it is put in RESULTBUF, and RESULTBUF is returned.
           Otherwise, a freshly allocated string is returned.  In both cases,
           ‘*LENGTHP’ is set to the length (number of units) of the returned
           string.  In case of error, NULL is returned and ‘errno’ is set."

  So, the functions that I would propose have the following prototype:
    char *str_from_time (char *resultbuf, size_t *lengthp,
                         bool i18n, const char *format,
                         time_t t,            bool local_time, int ns);
    char *str_from_tm   (char *resultbuf, size_t *lengthp,
                         bool i18n, const char *format,
                         struct tm const *tp, bool local_time, int ns);

  Migration from ctime:
    s = ctime (tp);
  =>
    char buf[64];
    size_t buf_size = sizeof (buf);
    s = str_from_time (buf, &buf_size, false, "%a %b %e %H:%M:%S %Y\n", *tp, true, 0);
    if (s == NULL) /* handle error case */;

  Migration from ctime_r:
    char buf[26];
    s = ctime_r (tp, buf);
  =>
    char buf[64];
    size_t buf_size = sizeof (buf);
    s = str_from_time (buf, &buf_size, false, "%a %b %e %H:%M:%S %Y\n", *tp, true, 0);
    if (s == NULL) /* handle error case */;

  Migration from asctime:
    s = asctime (tp);
  =>
    char buf[64];
    size_t buf_size = sizeof (buf);
    s = str_from_tm (buf, &buf_size, false, "%a %b %e %H:%M:%S %Y\n", tp, local_time, 0);
    if (s == NULL) /* handle error case */;

  Migration from asctime_r:
    char buf[26];
    s = asctime_r (tp, buf);
  =>
    char buf[64];
    size_t buf_size = sizeof (buf);
    s = str_from_tm (buf, &buf_size, false, "%a %b %e %H:%M:%S %Y\n", tp, local_time, 0);
    if (s == NULL) /* handle error case */;

* In which header file to declare the functions? I would propose <time.h>,
  since this makes migration from ctime(), ctime_r(), asctime(), asctime_r()
  easy.

Bruno





^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: Let's remove Gnulib's ctime module
  2024-02-08 17:00                 ` Bruno Haible
@ 2024-02-08 18:22                   ` Paul Eggert
  2024-02-08 19:20                     ` Simon Josefsson via Gnulib discussion list
  2024-02-09 15:00                     ` Bruno Haible
  0 siblings, 2 replies; 38+ messages in thread
From: Paul Eggert @ 2024-02-08 18:22 UTC (permalink / raw)
  To: Bruno Haible, bug-gnulib, Simon Josefsson

On 2024-02-08 09:00, Bruno Haible wrote:
>      char *str_from_time (char *resultbuf, size_t *lengthp,
>                           bool i18n, const char *format,
>                           time_t t,            bool local_time, int ns);
>      char *str_from_tm   (char *resultbuf, size_t *lengthp,
>                           bool i18n, const char *format,
>                           struct tm const *tp, bool local_time, int ns);

I suggest some simplifications and some complications:

1. There's no need for the from_tm flavor. That just complicates things, 
because callers use localtime or localtime_r or gmtime or gmtime_r or 
whatever, and deal with those other functions failing. Many programs 
don't do this correctly, so let's have str_from_done do it and be done 
with it.

2. Instead of i18n being a boolean, why not make it a locale_t? That's 
more general.

3. Likewise for localtime; why not make it a timezone_t? Sure it's 
overkill, but having a boolean at all is overkill in some sense.

4. We can also establish simpler APIs for common cases, which call the 
more-general API.

>   Migration from ctime:
>     s = ctime (tp);
>   =>
>     char buf[64];
>     size_t buf_size = sizeof (buf);
>     s = str_from_time (buf, &buf_size, false, "%a %b %e %H:%M:%S %Y\n", *tp, true, 0);
>     if (s == NULL) /* handle error case */;

The migration is more complicated than that, since one must have 'if (s 
!= buf) free (s);' afterwards.

Better to have struct timezone than separate time_t and ns args.

In practice I don't think these buffers will overflow, as time strings 
are short. So there's little need for malloc.

So how about the following API instead?

    size_t str_from_time (char *restrict s, idx_t maxsize,
                          char const *format, struct timespec t,
                          locale_t locale, timezone_t tz);

If localtime_rz fails, this substitutes the decimal representation of T. 
If the string doesn't fit this returns 0 and (if MAXSIZE is nonzero) 
stores some truncation of the string into S.

We also arrange for identifiers C_locale and local_tz to stand for
the C locale and the local timezone, and CTIME_BUFSIZE and ctime_fmt to 
be appropriate buffer sizes and formats for ctime-equivalent calls.

Migration from ctime:

   c = ctime (tp);
  =>
   char c[CTIME_BUFSIZE];
   str_from_time (c, sizeof c, ctime_fmt,
                  (struct timespec) { .tv_sec = *tp },
                  C_locale, local_tz);

and we can package this up into a function like this:

   char c[CTIME_BUFSIZE];
   safer_ctime (c, *tp);

if people prefer simplicity.




^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: Let's remove Gnulib's ctime module
  2024-02-08 18:22                   ` Paul Eggert
@ 2024-02-08 19:20                     ` Simon Josefsson via Gnulib discussion list
  2024-02-08 23:39                       ` Paul Eggert
                                         ` (2 more replies)
  2024-02-09 15:00                     ` Bruno Haible
  1 sibling, 3 replies; 38+ messages in thread
From: Simon Josefsson via Gnulib discussion list @ 2024-02-08 19:20 UTC (permalink / raw)
  To: Paul Eggert; +Cc: Bruno Haible, bug-gnulib

[-- Attachment #1: Type: text/plain, Size: 2994 bytes --]

Paul Eggert <eggert@cs.ucla.edu> writes:

> and we can package this up into a function like this:
>
>   char c[CTIME_BUFSIZE];
>   safer_ctime (c, *tp);
>
> if people prefer simplicity.

Yes please.  Using complex APIs to implement safer_ctime is fine, but I
would prefer to not make existing ctime code more complicated when
fixing the undefined problem with (as)ctime(_r).

I would prefer having a global static variant since several uses of
ctime is in non-thread global application context where even the above
is unnecessarily complicated and wasteful of stack space.

A term "safe" is subjective (safer than what?  does it preclude even
safer variants?), how about using the 'c_' prefix?

So unless I'm missing something I envision these new functions:

       #define CTIME_BUFSIZE

       char *c_asctime(const struct tm *tm);
       char *c_asctime_r(const struct tm *tm, char *buf);

       char *c_ctime(const time_t *timep);
       char *c_ctime_r(const time_t *timep, char *buf);

that would never trigger undefined behaviour due to the year with 64-bit
time_t, and maybe fix some other underspecified aspect of the existing
functions.

CTIME_BUFSIZE should allow room for all possible years for 64-bit time_t
when time_t is 64-bit, and all possible years for 128-bit time_t when
time_t is 128-bit...  support for sizeof (time_t) > 8 may be unsupported
by gnulib though -- I'm not sure any reasonable system will ever use
128-bit time_t, although it is conceivable that some system may not have
efficient integers less than 128-bit and then sizeof (time_t) == 16
would make sense, I guess.  If that is even permitted.

64-bit time_t signed is sufficient for years -292471206707
.. 292471210647 or something like that.  So I suppose the longest
possible strings we could see with 64-bit time_t would be this:

"Sun Sep 16 01:03:52 -292471206707\n\0"

Even a 64-bit unsigned year would fit in the same string:

1970+2^64/60/60/24/365 =  584942419325
1970+2^63/60/60/24/365 =  292471210647
1970-2^64/60/60/24/365 = -584942415385
1970-2^63/60/60/24/365 = -292471206707

So CTIME_BUFSIZE should be 35?

It would be a nice property that all possible time_t values can be
converted into some human readable string that can be converted back to
same time_t value, for the same time zone.

I don't think anyone will care strongly what the string format is as
long as it follows the "Sun Sep 16 01:03:52 1973\n\0" format for all
valid 32-bit time_t values.  Cut-off years for comparison:

1970-2^32/60/60/24/365 = 1834
1970-2^31/60/60/24/365 = 1902
1970+2^31/60/60/24/365 = 2038
1970+2^32/60/60/24/365 = 2106

Thanks Bruno for fixing the primitives!  They are important to have as
tooling for any higher level functions.

Btw, how does 'difftime' handle 64-bit time_t?  I suppose it depends on
size of double.  It seems difftime cannot return errors, and doesn't
document undefined behaviour for input values:
https://pubs.opengroup.org/onlinepubs/9699919799/functions/difftime.html

/Simon

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 255 bytes --]

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: Let's remove Gnulib's ctime module
  2024-02-08 19:20                     ` Simon Josefsson via Gnulib discussion list
@ 2024-02-08 23:39                       ` Paul Eggert
  2024-02-09  8:39                         ` Simon Josefsson via Gnulib discussion list
  2024-02-09 15:13                         ` Bruno Haible
  2024-02-09  0:20                       ` Paul Eggert
  2024-02-09 15:06                       ` Bruno Haible
  2 siblings, 2 replies; 38+ messages in thread
From: Paul Eggert @ 2024-02-08 23:39 UTC (permalink / raw)
  To: Simon Josefsson; +Cc: Bruno Haible, bug-gnulib

On 2/8/24 11:20, Simon Josefsson wrote:
> A term "safe" is subjective (safer than what?  does it preclude even
> safer variants?), how about using the 'c_' prefix?

That's why I used "safer_" rather than "safe_" - it's safer than ctime 
(though of course nothing is 100% safe).

"c_" is not appropriate because it means the C locale. ctime already 
means the C locale.

We could use some other prefix I suppose.

> So unless I'm missing something I envision these new functions:
> 
>         #define CTIME_BUFSIZE
> 
>         char *c_asctime(const struct tm *tm);
>         char *c_asctime_r(const struct tm *tm, char *buf);
> 
>         char *c_ctime(const time_t *timep);
>         char *c_ctime_r(const time_t *timep, char *buf);

Sounds a bit complicated...

Since asctime_r and ctime_r have already been removed from draft next 
POSIX, let's not name anything after them.

Why do we need asctime? It's just a recipe for trouble.

I'm hoping we can get by with just your c_ctime_r (but with a better 
name) and skip the other simple APIs. If someone something else they can 
use a more-detailed strftime-like API.


^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: Let's remove Gnulib's ctime module
  2024-02-08 19:20                     ` Simon Josefsson via Gnulib discussion list
  2024-02-08 23:39                       ` Paul Eggert
@ 2024-02-09  0:20                       ` Paul Eggert
  2024-02-09 15:06                       ` Bruno Haible
  2 siblings, 0 replies; 38+ messages in thread
From: Paul Eggert @ 2024-02-09  0:20 UTC (permalink / raw)
  To: Simon Josefsson; +Cc: Bruno Haible, bug-gnulib

On 2/8/24 11:20, Simon Josefsson wrote:
> I would prefer having a global static variant since several uses of
> ctime is in non-thread global application context where even the above
> is unnecessarily complicated and wasteful of stack space.

I suppose we could add a static variant too, though this wouldn't be a 
stack space issue (the stack space is minimal).

A key point, is that these replacement functions won't follow ctime 
rules all the time. You can't reliably get a year number by looking at 
four particular bytes in the output. Any code doing that now will need 
to be rewritten no matter what.


^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: obsolescent ctime
  2024-02-08  9:02       ` obsolescent ctime Bruno Haible
@ 2024-02-09  2:20         ` Paul Eggert
  0 siblings, 0 replies; 38+ messages in thread
From: Paul Eggert @ 2024-02-09  2:20 UTC (permalink / raw)
  To: Bruno Haible, bug-gnulib, Simon Josefsson

On 2/8/24 01:02, Bruno Haible wrote:

> https://codesearch.debian.net/search?q=%5Cb%28as%29%3Fctime%28_r%29%3F%5Cb+*%5B%28%5D+filetype%3Ac+package%3Acpio&literal=0

That URL is a false positive. The code's comment explains why:

   /* Get time values ready to print.  Do not worry about ctime failing,
      or a year outside the range 1000-9999, since 0 <= WHEN < 2**33.  */
   tbuf = ctime (&when);

I wrote that code last year when fixing some integer overflow bugs in 
GNU cpio <https://bugs.gnu.org/50694>.

The other URLs seem to be mostly true positives, some more serious since 
a file timestamp out of range can cause undefined behavior.


^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: Let's remove Gnulib's ctime module
  2024-02-08 23:39                       ` Paul Eggert
@ 2024-02-09  8:39                         ` Simon Josefsson via Gnulib discussion list
  2024-02-09  9:00                           ` Paul Eggert
  2024-02-09 15:13                         ` Bruno Haible
  1 sibling, 1 reply; 38+ messages in thread
From: Simon Josefsson via Gnulib discussion list @ 2024-02-09  8:39 UTC (permalink / raw)
  To: Paul Eggert; +Cc: Bruno Haible, bug-gnulib

[-- Attachment #1: Type: text/plain, Size: 2070 bytes --]

How about this (or gl-ctime?):

/* safer-ctime.h -- safer version of ctime().
   Copyright (C) 2024 FSF
   Authors: Paul Eggert, Bruno Haible, Simon Josefsson
   License: LGPL-2+
 */

#define SAFER_CTIME_BUFSIZE 35

/* Convert WHEN representing the number of seconds before/after epoch,
   1970-01-01 00:00:00 +0000 (UTC) to a fixed locale-independent
   NUL-terminated string such as "Wed Jun 30 21:49:08 1993\n\0",
   relative to the user's specified timezone, using abbreviations for
   the days of the week as "Sun", "Mon", "Tue", "Wed", "Thu", "Fri", and
   "Sat" and abbreviations for the months as "Jan", "Feb", "Mar", "Apr",
   "May", "Jun", "Jul", "Aug", "Sep", "Oct", "Nov", and "Dec".  The
   function does not set the external variables tzname, timezone or
   daylight, see tzset(3).  The output is copied into STR which should
   have room for at least SAFER_CTIME_BUFSIZE bytes.  For years 1000 to
   9999 inclusive the output string length will be 26 characters
   including the final NUL byte.  The string length may be shorter for
   years before 1000 and larger for years after 9999.  The years are not
   padded with whitespace or zeros, so valid outputs include strings
   "Wed Jun 30 21:49:08 623\n" and "Wed Jun 30 21:49:08 11147\n", and
   for negative years strings such as "Wed Jun 30 21:49:08 -42\n".  The
   preloptic Gregorian calendar is used for all years, to cover years
   before the Gregorian calendar was adopted; and for years before 1 the
   ISO 8601 approach to have years 2, 1, 0, -1, and so on is used
   instead of having 2 BC, 1 BC, AD 1, AD 2.  On systems with a 64-bit
   time_t type, the year value may be large as in strings looking like
   "Sun Sep 16 01:03:52 -292471206706\n\0", and future systems with
   larger time_t types may lead to even longer strings.  If WHEN cannot
   be converted into a string, NULL is returned and errno is set to an
   error, otherwise on success STR is returned.  The function's Thread
   safety attribute value is MT-Safe env locale. */

char *safer_ctime (time_t when, char *str);

/Simon

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 255 bytes --]

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: Let's remove Gnulib's ctime module
  2024-02-09  8:39                         ` Simon Josefsson via Gnulib discussion list
@ 2024-02-09  9:00                           ` Paul Eggert
  2024-02-09 15:22                             ` Bruno Haible
  2024-02-10 10:33                             ` Simon Josefsson via Gnulib discussion list
  0 siblings, 2 replies; 38+ messages in thread
From: Paul Eggert @ 2024-02-09  9:00 UTC (permalink / raw)
  To: Simon Josefsson; +Cc: Bruno Haible, bug-gnulib

On 2024-02-09 00:39, Simon Josefsson wrote:
> How about this (or gl-ctime?):

>     The
>     function does not set the external variables tzname, timezone or
>     daylight, see tzset(3).

I don't see how to implement that. We gotta call localtime or 
localtime_r and they can set those variables.

Anyway, nobody should care about those variables nowadays. See:

https://data.iana.org/time-zones/theory.html#vestigial


>     The string length may be shorter for
>     years before 1000 and larger for years after 9999.

The length can also be longer for years before -999.

>     If WHEN cannot
>     be converted into a string,

If we're going to this much work, let's always convert WHEN to a string, 
and never fail. This will simplify the caller, which is the point of 
this API.


>    The function's Thread
>     safety attribute value is MT-Safe env locale.

The function is locale-independent, so why say "locale" here?

When you write this sort of thing, do you mean the properties must be 
guaranteed on all platforms, or just on glibc?

Not sure it's worth the hassle of thinking out and documenting this 
stuff portably. Too much depends on the underlying implementation.


^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: Let's remove Gnulib's ctime module
  2024-02-08 18:22                   ` Paul Eggert
  2024-02-08 19:20                     ` Simon Josefsson via Gnulib discussion list
@ 2024-02-09 15:00                     ` Bruno Haible
  2024-02-10  4:43                       ` Paul Eggert
  1 sibling, 1 reply; 38+ messages in thread
From: Bruno Haible @ 2024-02-09 15:00 UTC (permalink / raw)
  To: bug-gnulib, Simon Josefsson, Paul Eggert

Paul Eggert wrote:
> >      char *str_from_time (char *resultbuf, size_t *lengthp,
> >                           bool i18n, const char *format,
> >                           time_t t,            bool local_time, int ns);
> >      char *str_from_tm   (char *resultbuf, size_t *lengthp,
> >                           bool i18n, const char *format,
> >                           struct tm const *tp, bool local_time, int ns);
> 
> I suggest some simplifications and some complications:
> 
> 1. There's no need for the from_tm flavor. That just complicates things, 
> because callers use localtime or localtime_r or gmtime or gmtime_r or 
> whatever

I agree that the majority of callers will use a time_t -> struct tm
conversion first. But if we want to recommend an 'asctime' and 'asctime_r'
replacement, we need to have a function that takes a 'struct tm const *'
argument.

> 2. Instead of i18n being a boolean, why not make it a locale_t? That's 
> more general.

We can't make it a locale_t because
  - Some platforms don't have it:
      native Windows (mingw, MSVC),
      FreeBSD < 9.0, NetBSD < 8.0, OpenBSD < 6.2,
      Solaris < 11.4, Cygwin < 2.6, Android API level < 21.
  - It would be quite complicated for gnulib to provide a locale_t.

> 3. Likewise for localtime; why not make it a timezone_t? Sure it's 
> overkill, but having a boolean at all is overkill in some sense.

'true' is simpler than 'tzalloc (getenv ("TZ"))'.

> 4. We can also establish simpler APIs for common cases, which call the 
> more-general API.

Sure, if we make it clear that these simpler APIs return strings in a global
buffer and are thus not MT-safe.

> >   Migration from ctime:
> >     s = ctime (tp);
> >   =>
> >     char buf[64];
> >     size_t buf_size = sizeof (buf);
> >     s = str_from_time (buf, &buf_size, false, "%a %b %e %H:%M:%S %Y\n", *tp, true, 0);
> >     if (s == NULL) /* handle error case */;
> 
> The migration is more complicated than that, since one must have 'if (s 
> != buf) free (s);' afterwards.

Not in this case, because when i18n == false, the given format string
cannot produce more than 64 characters. For i18n == true, you're right,
we should better recomment a larger buffer size, say 256.

> Better to have struct timezone than separate time_t and ns args.

You mean 'struct timespec'? That's a good idea for another variant:
  char *str_from_timespec (char *resultbuf, size_t *lengthp,
                           bool i18n, const char *format,
                           struct timespec t, bool local_time);

Not everyone find it simple to write
   (struct timespec) { .tv_sec = t, .tv_nsec = 0 }
Therefore the variant with a time_t argument will be appreciated by those
users who want to pass ns = 0.

> In practice I don't think these buffers will overflow, as time strings 
> are short. So there's little need for malloc.

One of the first occurrences that I found on codesearch.debian.net precisely
used malloc.

> If localtime_rz fails, this substitutes the decimal representation of T. 
> If the string doesn't fit this returns 0 and (if MAXSIZE is nonzero) 
> stores some truncation of the string into S.

I vehemently disagree. Storing truncated values has been a recipe for
malfunction in the past. The function should fail if it cannot produce
the expected result.

> We also arrange for identifiers C_locale and local_tz to stand for
> the C locale and the local timezone,

I would be OK with local_tz vs. gmt_tz as special values of timezone_t.
But local_tz would expand to a function call that caches the result,
otherwise we have a memory leak.

> and CTIME_BUFSIZE and ctime_fmt to 
> be appropriate buffer sizes and formats for ctime-equivalent calls.

OK with me. ctime_fmt could be a macro that expands to "%a %b %e %H:%M:%S %Y\n".

> and we can package this up into a function like this:
> 
>    char c[CTIME_BUFSIZE];
>    safer_ctime (c, *tp);
> 
> if people prefer simplicity.

Looks good to me. And a similar one as an 'asctime' replacement:

     char buf[CTIME_BUFSIZE];
     safer_asctime (buf, tp);

Bruno





^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: Let's remove Gnulib's ctime module
  2024-02-08 19:20                     ` Simon Josefsson via Gnulib discussion list
  2024-02-08 23:39                       ` Paul Eggert
  2024-02-09  0:20                       ` Paul Eggert
@ 2024-02-09 15:06                       ` Bruno Haible
  2024-02-10  4:22                         ` Paul Eggert
  2 siblings, 1 reply; 38+ messages in thread
From: Bruno Haible @ 2024-02-09 15:06 UTC (permalink / raw)
  To: Paul Eggert, Simon Josefsson; +Cc: bug-gnulib

Simon Josefsson wrote:
> I suppose the longest
> possible strings we could see with 64-bit time_t would be this:
> 
> "Sun Sep 16 01:03:52 -292471206707\n\0"
> 
> Even a 64-bit unsigned year would fit in the same string:
> 
> 1970+2^64/60/60/24/365 =  584942419325
> 1970+2^63/60/60/24/365 =  292471210647
> 1970-2^64/60/60/24/365 = -584942415385
> 1970-2^63/60/60/24/365 = -292471206707
> 
> So CTIME_BUFSIZE should be 35?

With 50 years of computer science experience, we should have learned
the lesson to allocate more room than sounds necessary *now*. If
now you think 35 will be sufficient for all times, then we should
better choose twice that value: 70.
 
Bruno





^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: Let's remove Gnulib's ctime module
  2024-02-08 23:39                       ` Paul Eggert
  2024-02-09  8:39                         ` Simon Josefsson via Gnulib discussion list
@ 2024-02-09 15:13                         ` Bruno Haible
  1 sibling, 0 replies; 38+ messages in thread
From: Bruno Haible @ 2024-02-09 15:13 UTC (permalink / raw)
  To: Simon Josefsson, Paul Eggert; +Cc: bug-gnulib

Paul Eggert wrote:
> "c_" is not appropriate because it means the C locale. ctime already 
> means the C locale.

Agree.

> Since asctime_r and ctime_r have already been removed from draft next 
> POSIX, let's not name anything after them.

Also because these names are not easily memorized. 'strftime' and
fprintftime is halfway reasonable, but 'ctime' and 'asctime' are not.

For the variants with more arguments,
  str_from_time
  str_from_timespec
  str_from_tm
sound good.

For the ones that operate in the C locale and are
meant as replacements for ctime and asctime, either
  safer_ctime
  safer_asctime
or
  c_strnl_from_time
  c_strnl_from_tm
would be OK with me. ("strnl" because they return a string that ends
in a newline.)

Bruno





^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: Let's remove Gnulib's ctime module
  2024-02-09  9:00                           ` Paul Eggert
@ 2024-02-09 15:22                             ` Bruno Haible
  2024-02-10 10:33                             ` Simon Josefsson via Gnulib discussion list
  1 sibling, 0 replies; 38+ messages in thread
From: Bruno Haible @ 2024-02-09 15:22 UTC (permalink / raw)
  To: Simon Josefsson, Paul Eggert; +Cc: bug-gnulib

> #define SAFER_CTIME_BUFSIZE 35

Make that 64. For future evolutions that we don't know about yet.

Paul Eggert wrote:
> When you write this sort of thing, do you mean the properties must be 
> guaranteed on all platforms, or just on glibc?
> 
> Not sure it's worth the hassle of thinking out and documenting this 
> stuff portably. Too much depends on the underlying implementation.

I agree. The doc should not say anything about "26 characters",
about the month names, about the Gregorian calendar, about years before AD,
etc. And instead of "The function's Thread safety attribute value is
MT-Safe env locale." (who understands that terminology?!) it should better
say "The result depends on the environment variable TZ."

Bruno





^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: Let's remove Gnulib's ctime module
  2024-02-09 15:06                       ` Bruno Haible
@ 2024-02-10  4:22                         ` Paul Eggert
  2024-02-10  9:53                           ` Bruno Haible
  0 siblings, 1 reply; 38+ messages in thread
From: Paul Eggert @ 2024-02-10  4:22 UTC (permalink / raw)
  To: Bruno Haible, Simon Josefsson; +Cc: bug-gnulib

On 2024-02-09 07:06, Bruno Haible wrote:
>> So CTIME_BUFSIZE should be 35?
> With 50 years of computer science experience, we should have learned
> the lesson to allocate more room than sounds necessary*now*. If
> now you think 35 will be sufficient for all times, then we should
> better choose twice that value: 70.

We needn't be that pessimistic. We can use something like this:

   #define CTIME_BUFSIZE \
     (sizeof "Wed Jun 30 21:49:08 \n" \
      + INT_STRLEN_BOUND (time_t) - 7)

7 = floor(log10(60*60*24*365)). This evaluates to 35 on typical machines 
today, and will grow automatically if time_t gets wider - it could even 
exceed 70 if needed (though I think this unlikely).


^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: Let's remove Gnulib's ctime module
  2024-02-09 15:00                     ` Bruno Haible
@ 2024-02-10  4:43                       ` Paul Eggert
  2024-02-10 10:03                         ` Bruno Haible
  0 siblings, 1 reply; 38+ messages in thread
From: Paul Eggert @ 2024-02-10  4:43 UTC (permalink / raw)
  To: Bruno Haible, bug-gnulib, Simon Josefsson

On 2024-02-09 07:00, Bruno Haible wrote:

> I agree that the majority of callers will use a time_t -> struct tm
> conversion first. But if we want to recommend an 'asctime' and 'asctime_r'
> replacement

We don't, because asctime is a bug magnet. Pretty much everybody who 
calls asctime calls localtime (or a relative) first, and almost 
everybody who does that neglects to check for localtime failure. It's a 
real mess, and we don't want to perpetuate the mess.

Much better to say: "just use ctime" (or some other substitute).


> We can't make it a locale_t because
>    - Some platforms don't have it:

That's OK. We provide a substitute "C_locale" that still works on these 
locales.

> 'true' is simpler than 'tzalloc (getenv ("TZ"))'.

It's not that much simpler than "localtime_tz", which is all callers 
need to know.

>> The migration is more complicated than that, since one must have 'if (s
>> != buf) free (s);' afterwards.
> 
> Not in this case, because when i18n == false, the given format string
> cannot produce more than 64 characters.

Surely we don't want callers to be thinking about this sort of thing 
routinely. This complexity is not needed.

> Not everyone find it simple to write
>     (struct timespec) { .tv_sec = t, .tv_nsec = 0 }

First, you don't need the ", tv_nsec = 0". Second, if it's too painful 
to write (struct timespec) { .tv_sec = t } we can supply a simple inline 
function "tspec (t)" that converts t to a struct timespec. Such a 
function would be useful elsewhere, I expect.


>> In practice I don't think these buffers will overflow, as time strings
>> are short. So there's little need for malloc.
> 
> One of the first occurrences that I found on codesearch.debian.net precisely
> used malloc.

I'm sure there will be a few callers that, for whatever reason, want to 
call malloc. That's OK; they can still do that. However, there's little 
reason for our functions to call malloc for them, because it's so rarely 
needed in practice when the buffers are so tiny, as they almost 
invariably are here.


>> If localtime_rz fails, this substitutes the decimal representation of T.
>> If the string doesn't fit this returns 0 and (if MAXSIZE is nonzero)
>> stores some truncation of the string into S.
> 
> I vehemently disagree.

OK, then if localtime_rz fails, ctime/str_from_time/whatever can figure 
out the year number anyway, and generate the correct string. This isn't 
that hard to do. Then there will be no excuse for failure for large 
time_t values. (This is another reason we want to discourage asctime 
flavors; you cannot do this sort of thing with asctime.)


> local_tz would expand to a function call that caches the result,
> otherwise we have a memory leak.

Another possibility is that it could expand ((timezone_t) 1) and the 
rest of the code could treat this specially. This would also prevent the 
memory leak.



^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: Let's remove Gnulib's ctime module
  2024-02-10  4:22                         ` Paul Eggert
@ 2024-02-10  9:53                           ` Bruno Haible
  2024-02-10 10:47                             ` Simon Josefsson via Gnulib discussion list
  0 siblings, 1 reply; 38+ messages in thread
From: Bruno Haible @ 2024-02-10  9:53 UTC (permalink / raw)
  To: Simon Josefsson, Paul Eggert; +Cc: bug-gnulib

Paul Eggert wrote:
> >> So CTIME_BUFSIZE should be 35?
> > With 50 years of computer science experience, we should have learned
> > the lesson to allocate more room than sounds necessary*now*. If
> > now you think 35 will be sufficient for all times, then we should
> > better choose twice that value: 70.
> 
> We needn't be that pessimistic. We can use something like this:
> 
>    #define CTIME_BUFSIZE \
>      (sizeof "Wed Jun 30 21:49:08 \n" \
>       + INT_STRLEN_BOUND (time_t) - 7)

This formula reflects *today*'s expectations. My point is that we
should be prepared for the unexpected events. Such as the U.S.
adopting the French Republican calendar with its longer month
names (germinal, brumaire, etc.). Or that the abbreviation and
padding habits change.

Bruno





^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: Let's remove Gnulib's ctime module
  2024-02-10  4:43                       ` Paul Eggert
@ 2024-02-10 10:03                         ` Bruno Haible
  2024-02-11  1:58                           ` Paul Eggert
  0 siblings, 1 reply; 38+ messages in thread
From: Bruno Haible @ 2024-02-10 10:03 UTC (permalink / raw)
  To: bug-gnulib, Simon Josefsson, Paul Eggert

Paul Eggert wrote:
> > I agree that the majority of callers will use a time_t -> struct tm
> > conversion first. But if we want to recommend an 'asctime' and 'asctime_r'
> > replacement
> 
> We don't, because asctime is a bug magnet. Pretty much everybody who 
> calls asctime calls localtime (or a relative) first, and almost 
> everybody who does that neglects to check for localtime failure.

That's something we should mention in doc/posix-functions/asctime{,_r}.texi.
I would not have realized that e.g. the groff change [1] introduced a bug in
src/libs/libgroff/curtime.cpp.

Bruno

[1] https://git.savannah.gnu.org/gitweb/?p=groff.git;a=commitdiff;h=d7bbfb04ea25a82a8597cdef6ebb391cb78ab47c





^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: Let's remove Gnulib's ctime module
  2024-02-09  9:00                           ` Paul Eggert
  2024-02-09 15:22                             ` Bruno Haible
@ 2024-02-10 10:33                             ` Simon Josefsson via Gnulib discussion list
  2024-02-11  1:50                               ` Paul Eggert
  1 sibling, 1 reply; 38+ messages in thread
From: Simon Josefsson via Gnulib discussion list @ 2024-02-10 10:33 UTC (permalink / raw)
  To: Paul Eggert; +Cc: Bruno Haible, bug-gnulib

[-- Attachment #1: Type: text/plain, Size: 4638 bytes --]

Another attempt below, with known open issues:

1) it seems something has to be said about tz variables, either the
function "always sets" them, "never sets" them, or (in new text below)
"may set" them depending on what other functions are called.  Not
optimal, but better than not documenting it.  We can inspect the
implementation later and change this too.

2) can we implement this in a way that it never fails?  I still allow
return==NULL to indicate errors below, until we can confirm that it is
possible to implement this in a way that cannot fail.  Returning "magic"
values like "1970-01-01" seems worse than NULL to me, since then callers
will need to do string comparisons to catch error situations.

3) below it says that nothing can be assumed about thread safety (beyond
that it depend on an environment variable), which seems a bit
sub-optimal, but let's see how this ends up being implemented and if we
can say something better.  Saying that nothing can be assumed about
thread safety is better than not saying anything, IMO.

4) Bruno suggested not documenting anything about week/month names, what
calendar is used, the year < 1 handling or expected output string
lengths -- I tend to disagree: at least my goal is for this function to
be a drop-in well-defined superset for ctime.  For ctime those
properties are either specified and documented (and then we want to
document that this function is compatible) or left
undefined/undocumented (and then we want to provide well-defined
portable documented semantics).  The later problem with ctime seems to
be the reason we need to introduce a safer variant in the first place.

5) Naming.  I'm okay with 'safer_ctime' but still think it is ugly.
Bruno's suggestion of 'c_strnl_from_time' sounds better to me, even
though it is a mouthful.  How about 'strtime.h' and 'strctime'?  If
there is a need to offer a drop-in for asctime, then 'strasctime' is
relevant.  This seems more in line with existing C stdlib functions.

/Simon

/* strtime.h -- safe versions of time-related string functions.
   Copyright (C) 2024 FSF
   Authors: Paul Eggert, Bruno Haible, Simon Josefsson
   License: LGPL-2+
 */

#include <stdint.h>
#include <time.h>

/* This evaluates to 35 on typical machines today, and will grow
   automatically if time_t gets wider - it could even exceed 70 if
   needed.  7 = floor(log10(60*60*24*365)). */
#define STRCTIME_BUFSIZE \
    (sizeof "Wed Jun 30 21:49:08 \n" \
     + INT_STRLEN_BOUND (time_t) - 7)

/* Convert WHEN representing the number of seconds related to epoch,
   1970-01-01 00:00:00 +0000 (UTC), to a fixed locale-independent
   NUL-terminated string such as "Wed Jun 30 21:49:08 1993\n\0",
   relative to the user's specified timezone (TZ environment variable),
   using abbreviations for the days of the week as "Sun", "Mon", "Tue",
   "Wed", "Thu", "Fri", and "Sat" and abbreviations for the months as
   "Jan", "Feb", "Mar", "Apr", "May", "Jun", "Jul", "Aug", "Sep", "Oct",
   "Nov", and "Dec".  The function may set the external variables
   tzname, timezone, and daylight (see tzset(3)) with information about
   the current timezone.  The output is copied into STR which should
   have room for at least STRCTIME_BUFSIZE bytes.  If STR is NULL, a
   pointer to a global statically pre-allocated buffer of size
   STRCTIME_BUFSIZE is used instead.  For years 1000 to 9999 inclusive
   the output string length is 26 characters including the final NUL
   byte.  The string length may be shorter for some years before 1000,
   and larger for years after 9999 or before -999.  The years are not
   padded with whitespace or zeros, so valid outputs include strings
   "Wed Jun 30 21:49:08 623\n" and "Wed Jun 30 21:49:08 11147\n", and
   for negative years strings such as "Wed Jun 30 21:49:08 -42\n".  The
   preloptic Gregorian calendar is used for all years, to cover years
   before the Gregorian calendar was adopted; and for years before 1 the
   ISO 8601 approach to have years 2, 1, 0, -1, and so on is used
   instead of having 2 BC, 1 BC, AD 1, AD 2.  On systems with a 64-bit
   time_t type, the year value may be large as in strings looking like
   "Sun Sep 16 01:03:52 -292471206706\n\0", and future systems with
   larger time_t types may lead to even longer strings.  If WHEN cannot
   be converted into a string, NULL is returned and errno is set to an
   error, otherwise on success STR (or a pointer to the global buffer)
   is returned.  The result depends on the environment variable TZ; no
   further Thread safety attributes can be reliably assumed about this
   function. */

char *strctime (time_t when, char *str);

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 255 bytes --]

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: Let's remove Gnulib's ctime module
  2024-02-10  9:53                           ` Bruno Haible
@ 2024-02-10 10:47                             ` Simon Josefsson via Gnulib discussion list
  2024-02-10 20:36                               ` Paul Eggert
  0 siblings, 1 reply; 38+ messages in thread
From: Simon Josefsson via Gnulib discussion list @ 2024-02-10 10:47 UTC (permalink / raw)
  To: Bruno Haible; +Cc: Paul Eggert, bug-gnulib

[-- Attachment #1: Type: text/plain, Size: 1496 bytes --]

Bruno Haible <bruno@clisp.org> writes:

> Paul Eggert wrote:
>> >> So CTIME_BUFSIZE should be 35?
>> > With 50 years of computer science experience, we should have learned
>> > the lesson to allocate more room than sounds necessary*now*. If
>> > now you think 35 will be sufficient for all times, then we should
>> > better choose twice that value: 70.
>> 
>> We needn't be that pessimistic. We can use something like this:
>> 
>>    #define CTIME_BUFSIZE \
>>      (sizeof "Wed Jun 30 21:49:08 \n" \
>>       + INT_STRLEN_BOUND (time_t) - 7)
>
> This formula reflects *today*'s expectations. My point is that we
> should be prepared for the unexpected events. Such as the U.S.
> adopting the French Republican calendar with its longer month
> names (germinal, brumaire, etc.). Or that the abbreviation and
> padding habits change.

I think support for those unexpected events belong in your more properly
designed str_to_time API family, rather than in a smallest safe
replacement for the existing US/English/Gregorian/ISO8601-centric poorly
designed ctime API, that for simplicity should continue to be
US/English/Gregorian/ISO8601-centric.

That means we ought to encourage people to consider the str_to_time API
when they fix existing occurances of ctime.  It seems some code will
need the predictable behaviour from the safer_ctime/strctime API I'm
thinking of, and some code will prefer a more human-friendly approach
that str_to_time can support.

/Simon

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 255 bytes --]

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: Let's remove Gnulib's ctime module
  2024-02-10 10:47                             ` Simon Josefsson via Gnulib discussion list
@ 2024-02-10 20:36                               ` Paul Eggert
  0 siblings, 0 replies; 38+ messages in thread
From: Paul Eggert @ 2024-02-10 20:36 UTC (permalink / raw)
  To: Simon Josefsson, Bruno Haible; +Cc: bug-gnulib

On 2024-02-10 02:47, Simon Josefsson wrote:
> I think support for those unexpected events belong in your more properly
> designed str_to_time API family, rather than in a smallest safe
> replacement for the existing US/English/Gregorian/ISO8601-centric poorly
> designed ctime API, that for simplicity should continue to be
> US/English/Gregorian/ISO8601-centric.

Exactly. Even if the US adopted the French Revolutionary calendar, 
safer_ctime (by whatever name) would not change because that would break 
its callers, which expect the traditional ctime format.

I doubt very much whether ISO C ctime would change either.


^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: Let's remove Gnulib's ctime module
  2024-02-10 10:33                             ` Simon Josefsson via Gnulib discussion list
@ 2024-02-11  1:50                               ` Paul Eggert
  0 siblings, 0 replies; 38+ messages in thread
From: Paul Eggert @ 2024-02-11  1:50 UTC (permalink / raw)
  To: Simon Josefsson; +Cc: Bruno Haible, bug-gnulib

On 2024-02-10 02:33, Simon Josefsson wrote:

> 2) can we implement this in a way that it never fails?  I still allow
> return==NULL to indicate errors below, until we can confirm that it is
> possible to implement this in a way that cannot fail.  Returning "magic"
> values like "1970-01-01" seems worse than NULL to me, since then callers
> will need to do string comparisons to catch error situations.

Yes, we can do it without magic string values. Our ctime substitute can 
call localtime_r only with timestamps where the resulting tm_year must 
be in int range. For arguments outside that range it can first reduce 
the time_t argument to a reasonable range, call localtime_r on the 
resulting in-range time_t value (this is guaranteed to succeed), and 
calculate the correct year using time_t arithmetic (this is guaranteed 
to not overflow), and convert the resulting year to a string by hand.


> my goal is for this function to
> be a drop-in well-defined superset for ctime.

If I understand you correctly, what you want is merely a well-defined 
implementation of ctime_r. That is, whenever POSIX.1-2017 says the 
behavior is undefined, you want an implementation that has well-defined 
and obviously correct behavior. If so, that solves the problem of your 
next point:


> 5) Naming.

Let's just call this replacement function 'ctime_r', because that's what 
it is. It doesn't matter that POSIX is obsoleting ctime_r, as this 
function is intended for use only for obsoletish callers. And it doesn't 
matter if the system ctime_r has undefined behavior in some cases, as 
our replacement will merely extend it so that the behavior is well-defined.



^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: Let's remove Gnulib's ctime module
  2024-02-10 10:03                         ` Bruno Haible
@ 2024-02-11  1:58                           ` Paul Eggert
  2024-02-11 10:16                             ` Bruno Haible
  0 siblings, 1 reply; 38+ messages in thread
From: Paul Eggert @ 2024-02-11  1:58 UTC (permalink / raw)
  To: Bruno Haible, bug-gnulib, Simon Josefsson

[-- Attachment #1: Type: text/plain, Size: 167 bytes --]

On 2024-02-10 02:03, Bruno Haible wrote:
> That's something we should mention in doc/posix-functions/asctime{,_r}.texi.

I gave that a shot by installing the attached.

[-- Attachment #2: 0001-doc-improve-warnings-about-ctime-etc.patch --]
[-- Type: text/x-patch, Size: 5588 bytes --]

From 3aafd55066902b942ebb0263a5c235e684e2eeca Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Sat, 10 Feb 2024 17:53:34 -0800
Subject: [PATCH] doc: improve warnings about ctime etc.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* doc/posix-functions/asctime.texi (asctime):
* doc/posix-functions/asctime_r.texi (asctime_r):
Say why these functions are typically used mistakenly.
* doc/posix-functions/ctime.texi (ctime):
* doc/posix-functions/ctime_r.texi (ctimef):
Say that they can dump core due to an internal null pointer, too.
* doc/posix-functions/ctime.texi (ctime):
Don’t recommend ctime_r.
---
 ChangeLog                          | 12 ++++++++++++
 doc/posix-functions/asctime.texi   |  6 ++++++
 doc/posix-functions/asctime_r.texi |  6 ++++++
 doc/posix-functions/ctime.texi     | 14 +++++++++-----
 doc/posix-functions/ctime_r.texi   |  4 ++++
 5 files changed, 37 insertions(+), 5 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 143100114a..8b6c726087 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,15 @@
+2024-02-10  Paul Eggert  <eggert@cs.ucla.edu>
+
+	doc: improve warnings about ctime etc.
+	* doc/posix-functions/asctime.texi (asctime):
+	* doc/posix-functions/asctime_r.texi (asctime_r):
+	Say why these functions are typically used mistakenly.
+	* doc/posix-functions/ctime.texi (ctime):
+	* doc/posix-functions/ctime_r.texi (ctimef):
+	Say that they can dump core due to an internal null pointer, too.
+	* doc/posix-functions/ctime.texi (ctime):
+	Don’t recommend ctime_r.
+
 2024-02-10  Bruno Haible  <bruno@clisp.org>
 
 	havelib: Add support for NetBSD/sparc64.
diff --git a/doc/posix-functions/asctime.texi b/doc/posix-functions/asctime.texi
index 13aae8385a..be2f3a6889 100644
--- a/doc/posix-functions/asctime.texi
+++ b/doc/posix-functions/asctime.texi
@@ -22,4 +22,10 @@ However, @code{strftime} is locale dependent.
 This function may overflow its internal buffer if its argument
 specifies a year before 1000 or after 9999.
 @xref{ctime}.
+@item
+Although it is tempting to call this function on the value that
+functions like @code{localtime} return, this is typically a mistake.
+On most current platforms, these functions return a null pointer for
+timestamps out of range, and this function has undefined behavior in
+that case.
 @end itemize
diff --git a/doc/posix-functions/asctime_r.texi b/doc/posix-functions/asctime_r.texi
index e948d3c47b..cf3ac4ce27 100644
--- a/doc/posix-functions/asctime_r.texi
+++ b/doc/posix-functions/asctime_r.texi
@@ -29,4 +29,10 @@ However, @code{strftime} is locale dependent.
 This function may overflow its output buffer if its argument
 specifies a year before 1000 or after 9999.
 @xref{ctime}.
+@item
+Although it is tempting to call this function on the value that
+functions like @code{localtime} return, this is typically a mistake.
+On most current platforms, these functions return a null pointer for
+timestamps out of range, and this function has undefined behavior in
+that case.
 @end itemize
diff --git a/doc/posix-functions/ctime.texi b/doc/posix-functions/ctime.texi
index 3a1aa489f4..3ed8e29840 100644
--- a/doc/posix-functions/ctime.texi
+++ b/doc/posix-functions/ctime.texi
@@ -26,12 +26,15 @@ However, @code{localtime_r} can fail and @code{strftime} is locale dependent.
 This function may overflow its internal buffer if its argument
 specifies a time before the year 1000 or after the year 9999.
 @item
+This function may dereference an internal null pointer if its argument
+specifies a time before the year @code{INT_MIN}+1900 or after the
+year @code{INT_MAX}+1900.
+@item
 The @code{ctime} function need not be reentrant, and consequently is
 not required to be thread safe.  Implementations of @code{ctime}
 typically write the timestamp into static buffer.  If two threads
 call @code{ctime} at roughly the same time, you might end up with the
-wrong date in one of the threads, or some undefined string.  There is
-a reentrant interface @code{ctime_r}.
+wrong date in one of the threads, or some undefined string.
 @item
 Native Windows platforms (mingw, MSVC) support only a subset of time
 zones supported by GNU or specified by POSIX@.  @xref{tzset}.
@@ -60,8 +63,9 @@ and so was OK for circa 1979 platforms.
 However, today's platforms have a @code{time_t} so wide
 that the year might not be in the range [1000, 9999].
 In this case the behavior of @code{ctime} is undefined
-and some platforms behave badly, overrunning a buffer;
-and even on platforms where no buffer overrun occurs,
-the 7th Edition code can generate wrong output for out-of-range years,
+and some platforms behave badly, overrunning a buffer
+or dereferencing an internal null pointer;
+and even on platforms where no undefined behavior occurs,
+the 7th Edition code generates wrong output for out-of-range years,
 because it incorrectly assumes that every year is represented by
 exactly four digits.
diff --git a/doc/posix-functions/ctime_r.texi b/doc/posix-functions/ctime_r.texi
index 8b3fcb4ea7..99ae828a54 100644
--- a/doc/posix-functions/ctime_r.texi
+++ b/doc/posix-functions/ctime_r.texi
@@ -30,4 +30,8 @@ However, @code{localtime_r} can fail and @code{strftime} is locale dependent.
 This function may overflow its output buffer if its argument
 specifies a time before the year 1000 or after the year 9999.
 @xref{ctime}.
+@item
+This function may dereference an internal null pointer if its argument
+specifies a time before the year @code{INT_MIN}+1900 or after the
+year @code{INT_MAX}+1900.
 @end itemize
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* Re: Let's remove Gnulib's ctime module
  2024-02-11  1:58                           ` Paul Eggert
@ 2024-02-11 10:16                             ` Bruno Haible
  0 siblings, 0 replies; 38+ messages in thread
From: Bruno Haible @ 2024-02-11 10:16 UTC (permalink / raw)
  To: bug-gnulib, Simon Josefsson, Paul Eggert

Paul Eggert wrote:
> > That's something we should mention in doc/posix-functions/asctime{,_r}.texi.
> 
> I gave that a shot by installing the attached.

Thanks!





^ permalink raw reply	[flat|nested] 38+ messages in thread

end of thread, other threads:[~2024-02-11 10:17 UTC | newest]

Thread overview: 38+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-02-03 13:25 lib/time.in.h, ctime has portability problems; with clang and groff Bjarni Ingi Gislason
2024-02-03 21:43 ` Paul Eggert
2024-02-03 21:46   ` Let's remove Gnulib's ctime module Paul Eggert
2024-02-05  8:16     ` Simon Josefsson via Gnulib discussion list
2024-02-05  8:59       ` Paul Eggert
2024-02-05  9:48         ` Simon Josefsson via Gnulib discussion list
2024-02-05 10:52           ` Simon Josefsson via Gnulib discussion list
2024-02-06  5:14             ` Paul Eggert
2024-02-06  8:04               ` Simon Josefsson via Gnulib discussion list
2024-02-08 17:00                 ` Bruno Haible
2024-02-08 18:22                   ` Paul Eggert
2024-02-08 19:20                     ` Simon Josefsson via Gnulib discussion list
2024-02-08 23:39                       ` Paul Eggert
2024-02-09  8:39                         ` Simon Josefsson via Gnulib discussion list
2024-02-09  9:00                           ` Paul Eggert
2024-02-09 15:22                             ` Bruno Haible
2024-02-10 10:33                             ` Simon Josefsson via Gnulib discussion list
2024-02-11  1:50                               ` Paul Eggert
2024-02-09 15:13                         ` Bruno Haible
2024-02-09  0:20                       ` Paul Eggert
2024-02-09 15:06                       ` Bruno Haible
2024-02-10  4:22                         ` Paul Eggert
2024-02-10  9:53                           ` Bruno Haible
2024-02-10 10:47                             ` Simon Josefsson via Gnulib discussion list
2024-02-10 20:36                               ` Paul Eggert
2024-02-09 15:00                     ` Bruno Haible
2024-02-10  4:43                       ` Paul Eggert
2024-02-10 10:03                         ` Bruno Haible
2024-02-11  1:58                           ` Paul Eggert
2024-02-11 10:16                             ` Bruno Haible
2024-02-08  9:02       ` obsolescent ctime Bruno Haible
2024-02-09  2:20         ` Paul Eggert
2024-02-05 14:37     ` Let's remove Gnulib's ctime module Bruno Haible
2024-02-06  6:59       ` Paul Eggert
2024-02-06  8:37         ` Bruno Haible
2024-02-05 14:02   ` lib/time.in.h, ctime has portability problems; with clang and groff Bruno Haible
2024-02-05 13:41 ` Bruno Haible
2024-02-06  5:27   ` Paul Eggert

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).