Re: Let's remove Gnulib's ctime module

[Bruno Haible]

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