This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH v2 0/9] Y2038 struct tm related patches
- From: Albert ARIBAUD <albert dot aribaud at 3adev dot fr>
- To: Joseph Myers <joseph at codesourcery dot com>
- Cc: <libc-alpha at sourceware dot org>
- Date: Wed, 5 Dec 2018 19:40:59 +0100
- Subject: Re: [PATCH v2 0/9] Y2038 struct tm related patches
- References: <20181129221010.19589-1-albert.aribaud@3adev.fr> <alpine.DEB.2.21.1812051634060.27434@digraph.polyomino.org.uk>
Hi Joseph,
On Wed, 5 Dec 2018 16:49:26 +0000, Joseph Myers
<joseph@codesourcery.com> wrote :
> Patch 1 is OK.
Thanks! Accordingly, I'll apply this patch on master and re-spin the
series above it.
> For most of the rest of the series, I think there is a general issue to
> resolve regarding how we document (internally) the symbol handling for
> time-related symbols. In patch 5, for example, you have:
>
> > +/* This always works because either __TIMESIZE != 64 and __gmtime_r exists
> > + or __TIMESIZE == 64 and the definition of __gmtime64_r above actually
> > + defined __gmtime_r. */
>
> But this is nothing to do with those particular symbols; it's completely
> generic, as part of the way time-related symbols are handled in this
> series, that each such function for time_t is either defined explicitly,
> or via a #define of the name used with __time64_t. So it doesn't deserve
> a comment there, any more than each use of libc_hidden_def or weak_alias
> deserves a comment on the generic symbol handling conventions in glibc
> that result in those uses (only those with something unusual about them
> should have such comments).
>
> Now, those conventions probably *do* deserve to be documented somewhere
> (in the glibc sources, not just on the wiki), but somewhere not tied to
> one particular function. I think a better place might be maint.texi: add
> a section "Symbol Handling" or similar, which initially would just discuss
> how symbols for 64-bit time are handled (both on systems that had it all
> along, and on systems where support is being added alongside existing
> support for 32-bit time), but could be expanded in future to discuss the
> various other symbol handling issues in glibc.
I will prepend a patch to the (re-spun) series to add a section
"Symbol handling" to maint.texi, with a subsection "Y2038 symbol
handling" in which I will document these conventions.
> Patch 6 has a similarly problematic comment:
>
> > +/* nis/nis_print.c needs ctime, so even if ctime is not declared here,
> > + we define __ctime64 as ctime so that nis/nis_print.c can get linked
> > + against a function called ctime. */
>
> (a) __ctime64 is defined as ctime as part of the general symbol
> conventions for such symbols, since ctime is unconditionally part of the
> glibc ABI and API. It's nothing to do with a particular use in
> nis/nis_print.c. (Once that comment is accordingly removed, no further
> action is needed for (b) and (c) below; they are just general observations
> on related issues.)
>
> (b) Any such non-obvious reference in a comment from one part of the glibc
> source to another really needs a corresponding comment pointing in the
> other direction, so that someone changing nis/nis_print.c would know they
> might need to update the other comment.
>
> (c) In general, code internal to glibc should change to use 64-bit time
> explicitly rather than the old time_t / ctime. In this particular case,
> (i) it probably can't do so yet because you can't changes users outside of
> libc.so until libc.so exports the new interfaces, and (ii) the change
> might not provide much benefit, if the 32-bit times are part of the NIS
> protocol (which would mean NIS is obsolete by Y2038) - although it may be
> a good idea anyway to get rid of glibc-internal uses of the 32-bit
> interfaces.
Understood.
Cordialement,
Albert ARIBAUD
3ADEV