This is the mail archive of the
mailing list for the glibc project.
Re: [PATCH v2] Ensure mktime sets errno on error (bug 23789)
- From: Paul Eggert <eggert at cs dot ucla dot edu>
- To: "Albert ARIBAUD (3ADEV)" <albert dot aribaud at 3adev dot fr>, libc-alpha at sourceware dot org
- Date: Mon, 29 Oct 2018 00:05:12 -0700
- Subject: Re: [PATCH v2] Ensure mktime sets errno on error (bug 23789)
- References: <email@example.com>
Albert ARIBAUD (3ADEV) wrote:
+ /* record errno from __tzset() but do not fail now. */
+ errno_from_tzset = errno;
Why not fail now? If tzset fails, mktime is pointless.
Also, remember this code is shared with Gnulib, and POSIX does not specify what
tzset does to errno. So you can't rely on tzset setting errno, at least not in
the Gnulib usage of this code. And can you even rely on glibc __tzset doing it?
It's not part of tzset's spec, surely.
I thought the idea was to define a glibc-specific tzset variant that returned an
error number, or something like that. I don't see that here.
Also, how about moving the tzset call into the # if defined _LIBC ||
NEED_MKTIME_WORKING branch? tzset shouldn't be needed otherwise.
+ if (result == -1 && errno == 0)
+ return result;> + errno = 0;
+ else if (errno != 0)
+ return -1;
+ else if (errno_from_tzset != 0)
+ return result;
Sorry, but this is all too complicated. Remind me again why we can't rely on
__mktime_internal to set errno on failure.
We don't care what happens to errno when __mktime_internal returns any value
other than -1. Does that help?
I was hoping that mktime itself could look something like the following, which
would be a lot simpler. Of course __tzset_with_result would have to be written
(both in the Gnulib and the glibc case; the Gnulib version is easy), and
__mktime_internal would have to do the right thing with errno, but this is the idea.
mktime (struct tm *tp)
int r = __tzset_with_result ();
if (r < 0)
# if defined _LIBC || NEED_MKTIME_WORKING
static mktime_offset_t localtime_offset;
return __mktime_internal (tp, __localtime_r, &localtime_offset);
# undef mktime
return mktime (tp);