This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH v2] Ensure mktime sets errno on error (bug 23789)


Albert ARIBAUD (3ADEV) wrote:

    __tzset ();
+  /* 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)
+    __set_errno(errno_from_tzset);
+  else
+    __set_errno(errno_before_mktime);
+  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.

time_t
mktime (struct tm *tp)
{
  int r = __tzset_with_result ();
  if (r < 0)
    return r;

# if defined _LIBC || NEED_MKTIME_WORKING
  static mktime_offset_t localtime_offset;
  return __mktime_internal (tp, __localtime_r, &localtime_offset);
# else
#  undef mktime
  return mktime (tp);
# endif
}


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]