[1/5] Y2038: add function __localtime64_r

Message ID 20181217220429.4599-2-albert.aribaud@3adev.fr
State Superseded
Headers show
Series
  • Y2038: process remaining struct tm conv functions
Related show

Commit Message

Albert ARIBAUD (3ADEV) Dec. 17, 2018, 10:04 p.m.
Tested with 'make check' on x86_64-linux-gnu and i686-linux.gnu.

	* include/time.h
	(__localtime64_r): Add.
	* time/localtime.c
	(__localtime64_r): Add.
	[__TIMESIZE != 64] (__localtime_r): Turn into a wrapper.
---
 include/time.h   |  7 +++++++
 time/localtime.c | 16 +++++++++++++++-
 2 files changed, 22 insertions(+), 1 deletion(-)

-- 
2.17.1

Comments

Joseph Myers Dec. 17, 2018, 10:17 p.m. | #1
On Mon, 17 Dec 2018, Albert ARIBAUD (3ADEV) wrote:

> +/* Provide a 32-bit variant if needed */


Throughout this series, please make sure comments end with ".  " (full 
stop, two spaces) before end of comment.  You have the same mistake in 
other patches in this series.

You're declaring __localtime64_r with attribute_hidden.  Although not 
strictly wrong at this stage, since the point is eventually to export it 
from libc it would be better to declare it without attribute_hidden, and 
instead use libc_hidden_proto / libc_hidden_def (those are needed because 
of the call from __localtime_r).  Otherwise you'll need to remove the 
attribute_hidden, and add the libc_hidden_*, in a later patch that exports 
or prepares to export the functions.

-- 
Joseph S. Myers
joseph@codesourcery.com
Albert ARIBAUD (3ADEV) Dec. 18, 2018, 9:16 a.m. | #2
Hi Joseph,

On Mon, 17 Dec 2018 22:17:22 +0000, Joseph Myers
<joseph@codesourcery.com> wrote :

> On Mon, 17 Dec 2018, Albert ARIBAUD (3ADEV) wrote:

> 

> > +/* Provide a 32-bit variant if needed */  

> 

> Throughout this series, please make sure comments end with ".  " (full 

> stop, two spaces) before end of comment.  You have the same mistake in 

> other patches in this series.


Fixed. Is there a script that I could run on my local repo that
would check for these and which I could install as a git pre-commit
hook?

> You're declaring __localtime64_r with attribute_hidden.  Although not 

> strictly wrong at this stage, since the point is eventually to export it 

> from libc it would be better to declare it without attribute_hidden, and 

> instead use libc_hidden_proto / libc_hidden_def (those are needed because 

> of the call from __localtime_r).  Otherwise you'll need to remove the 

> attribute_hidden, and add the libc_hidden_*, in a later patch that exports 

> or prepares to export the functions.


Fixed for __localtime64_r, __gmtime64 and __gmtime64_r.

ctime and ctime_r are not called from within glibc at all and have no
libc_hidden_proto/libc_hidden_def, so I did not add any to __ctime64
and __ctime64_r.

Thanks for your feedback!

Cordialement,
Albert ARIBAUD
3ADEV
Albert ARIBAUD (3ADEV) Dec. 18, 2018, 10:10 a.m. | #3
On Tue, 18 Dec 2018 10:16:51 +0100, Albert ARIBAUD
<albert.aribaud@3adev.fr> wrote :

> ctime and ctime_r are not called from within glibc at all and have no

> libc_hidden_proto/libc_hidden_def, so I did not add any to __ctime64

> and __ctime64_r.


... but since __ctime64 is called by ctime and __ctime64_r is called by
ctime_r, in fact __ctime64 and __time64_r do need libc_hidden_proto and
libc_hidden_def.

Cordialement,
Albert ARIBAUD
3ADEV
Joseph Myers Dec. 18, 2018, 5:18 p.m. | #4
On Tue, 18 Dec 2018, Albert ARIBAUD wrote:

> > On Mon, 17 Dec 2018, Albert ARIBAUD (3ADEV) wrote:

> > 

> > > +/* Provide a 32-bit variant if needed */  

> > 

> > Throughout this series, please make sure comments end with ".  " (full 

> > stop, two spaces) before end of comment.  You have the same mistake in 

> > other patches in this series.

> 

> Fixed. Is there a script that I could run on my local repo that

> would check for these and which I could install as a git pre-commit

> hook?


I am not aware of such a script.

-- 
Joseph S. Myers
joseph@codesourcery.com

Patch

diff --git a/include/time.h b/include/time.h
index 3bc303a36e..34368295a9 100644
--- a/include/time.h
+++ b/include/time.h
@@ -67,6 +67,13 @@  libc_hidden_proto (__localtime64)
 extern struct tm *__localtime_r (const time_t *__timer,
 				 struct tm *__tp) attribute_hidden;
 
+#if __TIMESIZE == 64
+# define __localtime64_r __localtime_r
+#else
+extern struct tm *__localtime64_r (const __time64_t *__timer,
+				   struct tm *__tp) attribute_hidden;
+#endif
+
 extern struct tm *__gmtime_r (const time_t *__restrict __timer,
 			      struct tm *__restrict __tp);
 libc_hidden_proto (__gmtime_r)
diff --git a/time/localtime.c b/time/localtime.c
index 96879d4ec0..37169234c8 100644
--- a/time/localtime.c
+++ b/time/localtime.c
@@ -25,10 +25,24 @@  struct tm _tmbuf;
 /* Return the `struct tm' representation of *T in local time,
    using *TP to store the result.  */
 struct tm *
-__localtime_r (const time_t *t, struct tm *tp)
+__localtime64_r (const __time64_t *t, struct tm *tp)
 {
   return __tz_convert (*t, 1, tp);
 }
+
+/* Provide a 32-bit variant if needed */
+
+#if __TIMESIZE != 64
+
+struct tm *
+__localtime_r (const time_t *t, struct tm *tp)
+{
+  __time64_t t64 = *t;
+  return __localtime64_r (&t64, tp);
+}
+
+#endif
+
 weak_alias (__localtime_r, localtime_r)