Pedro Alves has posted comments on this change.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/613
......................................................................
Patch Set 1: Code-Review+2
(2 comments)
LGTM, with the nit pointed out below addressed.
| --- /dev/null
| +++ /COMMIT_MSG
| @@ -1,0 +2,19 @@ Parent: 66622f21 (Import the strerror_r-posix module and use it in GDB.)
| +Author: Christian Biesinger <cbiesinger@google.com>
| +AuthorDate: 2019-11-08 11:25:17 -0600
| +Commit: Christian Biesinger <cbiesinger@google.com>
| +CommitDate: 2019-11-11 16:20:34 -0600
| +
| +Import the time_r gnulib module
| +
| +This allows GDB to use localtime_r unconditionally.
| +
| +See https://lists.gnu.org/archive/html/bug-gnulib/2019-11/msg00017.html
PS1, Line 11:
Please mention this in the comment in the code as well. Otherwise
anyone staring at:
> /* Must be included before pathmax.h. */
> #include <time.h>
in the future will wonder why must that be so. Hopefully, at some
point we'll import a gnulib that has itself a workaround for this.
Otherwise LGTM.
| +for details on the compile error mentioned below.
| +
| +gdb/ChangeLog:
| +
| +2019-11-11 Christian Biesinger <cbiesinger@google.com>
| +
| + * gdbsupport/common-defs.h: Include time.h before pathmax.h to
| + avoid compile errors.
| +
| --- gdb/gdbsupport/common-defs.h
| +++ gdb/gdbsupport/common-defs.h
| @@ -91,17 +91,20 @@ #include <stdio.h>
| #include <stdlib.h>
| #include <stddef.h>
| #include <stdint.h>
| #include <string.h>
| #ifdef HAVE_STRINGS_H
| #include <strings.h> /* for strcasecmp and strncasecmp */
| #endif
| #include <errno.h>
| #include <alloca.h>
| +/* Must be included before pathmax.h. */
| +#include <time.h>
PS1, Line 101:
I don't think we know yet why this works around the issue, but I think
I'm fine with that. If we import a gnulib fix, then we may consider
only including time.h where is it necessary.
| +
|
| #include "ansidecl.h"
| /* This is defined by ansidecl.h, but we prefer gnulib's version. On
| MinGW, gnulib might enable __USE_MINGW_ANSI_STDIO, which may or not
| require use of attribute gnu_printf instead of printf. gnulib
| checks that at configure time. Since _GL_ATTRIBUTE_FORMAT_PRINTF
| is compatible with ATTRIBUTE_PRINTF, simply use it. */
| #undef ATTRIBUTE_PRINTF
@@ -97,6 +97,9 @@
#endif
#include <errno.h>
#include <alloca.h>
+/* Must be included before pathmax.h. */
+#include <time.h>
+
#include "ansidecl.h"
/* This is defined by ansidecl.h, but we prefer gnulib's version. On
@@ -135,6 +135,7 @@
import/m4/tempname.m4 \
import/m4/threadlib.m4 \
import/m4/time_h.m4 \
+ import/m4/time_r.m4 \
import/m4/unistd-safer.m4 \
import/m4/unistd_h.m4 \
import/m4/warn-on-use.m4 \
@@ -1848,6 +1848,7 @@
m4_include([import/m4/tempname.m4])
m4_include([import/m4/threadlib.m4])
m4_include([import/m4/time_h.m4])
+m4_include([import/m4/time_r.m4])
m4_include([import/m4/unistd-safer.m4])
m4_include([import/m4/unistd_h.m4])
m4_include([import/m4/warn-on-use.m4])
@@ -273,6 +273,9 @@
/* Define to 1 when the gnulib module strtok_r should be tested. */
#undef GNULIB_TEST_STRTOK_R
+/* Define to 1 when the gnulib module time_r should be tested. */
+#undef GNULIB_TEST_TIME_R
+
/* Define to 1 when the gnulib module unsetenv should be tested. */
#undef GNULIB_TEST_UNSETENV
@@ -346,6 +349,10 @@
don't. */
#undef HAVE_DECL_ISBLANK
+/* Define to 1 if you have the declaration of `localtime_r', and to 0 if you
+ don't. */
+#undef HAVE_DECL_LOCALTIME_R
+
/* Define to 1 if you have the declaration of `mbrtowc', and to 0 if you
don't. */
#undef HAVE_DECL_MBRTOWC
@@ -523,6 +530,9 @@
/* Define to 1 if you have the 'link' function. */
#undef HAVE_LINK
+/* Define to 1 if you have the 'localtime_r' function. */
+#undef HAVE_LOCALTIME_R
+
/* Define to 1 if the system has the type 'long long int'. */
#undef HAVE_LONG_LONG_INT
@@ -3564,6 +3564,7 @@
gl_func_list="$gl_func_list catgets"
gl_func_list="$gl_func_list snprintf"
gl_header_list="$gl_header_list sys/uio.h"
+gl_func_list="$gl_func_list localtime_r"
gl_func_list="$gl_func_list pipe"
gl_func_list="$gl_func_list iswcntrl"
# Check that the precious variables saved in the cache have kept the same
@@ -5984,6 +5985,7 @@
# Code from module time:
+ # Code from module time_r:
# Code from module unistd:
# Code from module unistd-safer:
# Code from module unsetenv:
@@ -16077,6 +16079,11 @@
+
+
+
+
+
ac_fn_c_check_decl "$LINENO" "unsetenv" "ac_cv_have_decl_unsetenv" "$ac_includes_default"
if test "x$ac_cv_have_decl_unsetenv" = xyes; then :
ac_have_decl=1
@@ -27016,6 +27023,110 @@
+ ac_fn_c_check_decl "$LINENO" "localtime_r" "ac_cv_have_decl_localtime_r" "#include <time.h>
+"
+if test "x$ac_cv_have_decl_localtime_r" = xyes; then :
+ ac_have_decl=1
+else
+ ac_have_decl=0
+fi
+
+cat >>confdefs.h <<_ACEOF
+#define HAVE_DECL_LOCALTIME_R $ac_have_decl
+_ACEOF
+
+ if test $ac_cv_have_decl_localtime_r = no; then
+ HAVE_DECL_LOCALTIME_R=0
+ fi
+
+
+ :
+
+
+
+
+
+ if test $ac_cv_func_localtime_r = yes; then
+ HAVE_LOCALTIME_R=1
+ { $as_echo "$as_me:${as_lineno-$LINENO}: checking whether localtime_r is compatible with its POSIX signature" >&5
+$as_echo_n "checking whether localtime_r is compatible with its POSIX signature... " >&6; }
+if ${gl_cv_time_r_posix+:} false; then :
+ $as_echo_n "(cached) " >&6
+else
+ cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h. */
+#include <time.h>
+int
+main ()
+{
+/* We don't need to append 'restrict's to the argument types,
+ even though the POSIX signature has the 'restrict's,
+ since C99 says they can't affect type compatibility. */
+ struct tm * (*ptr) (time_t const *, struct tm *) = localtime_r;
+ if (ptr) return 0;
+ /* Check the return type is a pointer.
+ On HP-UX 10 it is 'int'. */
+ *localtime_r (0, 0);
+ ;
+ return 0;
+}
+
+_ACEOF
+if ac_fn_c_try_compile "$LINENO"; then :
+ gl_cv_time_r_posix=yes
+else
+ gl_cv_time_r_posix=no
+fi
+rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
+
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $gl_cv_time_r_posix" >&5
+$as_echo "$gl_cv_time_r_posix" >&6; }
+ if test $gl_cv_time_r_posix = yes; then
+ REPLACE_LOCALTIME_R=0
+ else
+ REPLACE_LOCALTIME_R=1
+ fi
+ else
+ HAVE_LOCALTIME_R=0
+ fi
+
+ if test $HAVE_LOCALTIME_R = 0 || test $REPLACE_LOCALTIME_R = 1; then
+
+
+
+
+
+
+
+
+ gl_LIBOBJS="$gl_LIBOBJS time_r.$ac_objext"
+
+
+ :
+
+ fi
+
+
+
+
+
+ GNULIB_TIME_R=1
+
+
+
+
+
+$as_echo "#define GNULIB_TEST_TIME_R 1" >>confdefs.h
+
+
+
+
+
+
+
+
+
@@ -21,7 +21,7 @@
# the same distribution terms as the rest of that program.
#
# Generated by gnulib-tool.
-# Reproduce by: gnulib-tool --import --lib=libgnu --source-base=import --m4-base=import/m4 --doc-base=doc --tests-base=tests --aux-dir=import/extra --no-conditional-dependencies --no-libtool --macro-prefix=gl --no-vc-files alloca canonicalize-lgpl dirent dirfd errno fnmatch-gnu frexpl getcwd glob inet_ntop inttypes limits-h lstat memchr memmem mkdir mkdtemp mkostemp pathmax rawmemchr readlink rename setenv signal-h strchrnul strerror_r-posix strstr strtok_r sys_stat unistd unsetenv update-copyright wchar wctype-h
+# Reproduce by: gnulib-tool --import --lib=libgnu --source-base=import --m4-base=import/m4 --doc-base=doc --tests-base=tests --aux-dir=import/extra --no-conditional-dependencies --no-libtool --macro-prefix=gl --no-vc-files alloca canonicalize-lgpl dirent dirfd errno fnmatch-gnu frexpl getcwd glob inet_ntop inttypes limits-h lstat memchr memmem mkdir mkdtemp mkostemp pathmax rawmemchr readlink rename setenv signal-h strchrnul strerror_r-posix strstr strtok_r sys_stat time_r unistd unsetenv update-copyright wchar wctype-h
AUTOMAKE_OPTIONS = 1.9.6 gnits subdir-objects
@@ -2379,6 +2379,15 @@
## end gnulib module time
+## begin gnulib module time_r
+
+
+EXTRA_DIST += time_r.c
+
+EXTRA_libgnu_a_SOURCES += time_r.c
+
+## end gnulib module time_r
+
## begin gnulib module unistd
BUILT_SOURCES += unistd.h
@@ -35,7 +35,7 @@
# the same distribution terms as the rest of that program.
#
# Generated by gnulib-tool.
-# Reproduce by: gnulib-tool --import --lib=libgnu --source-base=import --m4-base=import/m4 --doc-base=doc --tests-base=tests --aux-dir=import/extra --no-conditional-dependencies --no-libtool --macro-prefix=gl --no-vc-files alloca canonicalize-lgpl dirent dirfd errno fnmatch-gnu frexpl getcwd glob inet_ntop inttypes limits-h lstat memchr memmem mkdir mkdtemp mkostemp pathmax rawmemchr readlink rename setenv signal-h strchrnul strerror_r-posix strstr strtok_r sys_stat unistd unsetenv update-copyright wchar wctype-h
+# Reproduce by: gnulib-tool --import --lib=libgnu --source-base=import --m4-base=import/m4 --doc-base=doc --tests-base=tests --aux-dir=import/extra --no-conditional-dependencies --no-libtool --macro-prefix=gl --no-vc-files alloca canonicalize-lgpl dirent dirfd errno fnmatch-gnu frexpl getcwd glob inet_ntop inttypes limits-h lstat memchr memmem mkdir mkdtemp mkostemp pathmax rawmemchr readlink rename setenv signal-h strchrnul strerror_r-posix strstr strtok_r sys_stat time_r unistd unsetenv update-copyright wchar wctype-h
@@ -252,6 +252,7 @@
$(top_srcdir)/import/m4/tempname.m4 \
$(top_srcdir)/import/m4/threadlib.m4 \
$(top_srcdir)/import/m4/time_h.m4 \
+ $(top_srcdir)/import/m4/time_r.m4 \
$(top_srcdir)/import/m4/unistd-safer.m4 \
$(top_srcdir)/import/m4/unistd_h.m4 \
$(top_srcdir)/import/m4/warn-on-use.m4 \
@@ -1553,8 +1554,8 @@
strerror_r.c string.in.h str-two-way.h strstr.c strtok_r.c \
sys_socket.in.h sys_stat.in.h sys_time.in.h sys_types.in.h \
sys_uio.in.h tempname.h \
- $(top_srcdir)/import/extra/config.rpath time.in.h unistd.in.h \
- unistd--.h unistd-safer.h unsetenv.c \
+ $(top_srcdir)/import/extra/config.rpath time.in.h time_r.c \
+ unistd.in.h unistd--.h unistd-safer.h unsetenv.c \
$(top_srcdir)/import/extra/update-copyright verify.h \
wchar.in.h wctype.in.h
@@ -1617,7 +1618,7 @@
readlink.c realloc.c rename.c rewinddir.c rmdir.c \
secure_getenv.c setenv.c stat.c strchrnul.c strdup.c \
strerror.c strerror-override.c strerror_r.c strstr.c \
- strtok_r.c unsetenv.c
+ strtok_r.c time_r.c unsetenv.c
# Use this preprocessor expression to decide whether #include_next works.
# Do not rely on a 'configure'-time test for this, since the expression
@@ -1791,6 +1792,7 @@
@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/strtok_r.Po@am__quote@
@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/sys_socket.Po@am__quote@
@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/tempname.Po@am__quote@
+@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/time_r.Po@am__quote@
@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/unistd.Po@am__quote@
@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/unsetenv.Po@am__quote@
@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/wctype-h.Po@am__quote@
@@ -27,7 +27,7 @@
# Specification in the form of a command-line invocation:
-# gnulib-tool --import --lib=libgnu --source-base=import --m4-base=import/m4 --doc-base=doc --tests-base=tests --aux-dir=import/extra --no-conditional-dependencies --no-libtool --macro-prefix=gl --no-vc-files alloca canonicalize-lgpl dirent dirfd errno fnmatch-gnu frexpl getcwd glob inet_ntop inttypes limits-h lstat memchr memmem mkdir mkdtemp mkostemp pathmax rawmemchr readlink rename setenv signal-h strchrnul strerror_r-posix strstr strtok_r sys_stat unistd unsetenv update-copyright wchar wctype-h
+# gnulib-tool --import --lib=libgnu --source-base=import --m4-base=import/m4 --doc-base=doc --tests-base=tests --aux-dir=import/extra --no-conditional-dependencies --no-libtool --macro-prefix=gl --no-vc-files alloca canonicalize-lgpl dirent dirfd errno fnmatch-gnu frexpl getcwd glob inet_ntop inttypes limits-h lstat memchr memmem mkdir mkdtemp mkostemp pathmax rawmemchr readlink rename setenv signal-h strchrnul strerror_r-posix strstr strtok_r sys_stat time_r unistd unsetenv update-copyright wchar wctype-h
# Specification in the form of a few gnulib-tool.m4 macro invocations:
gl_LOCAL_DIR([])
@@ -61,6 +61,7 @@
strstr
strtok_r
sys_stat
+ time_r
unistd
unsetenv
update-copyright
@@ -182,6 +182,7 @@
# Code from module threadlib:
gl_THREADLIB_EARLY
# Code from module time:
+ # Code from module time_r:
# Code from module unistd:
# Code from module unistd-safer:
# Code from module unsetenv:
@@ -619,6 +620,12 @@
gl_FUNC_GEN_TEMPNAME
gl_THREADLIB
gl_HEADER_TIME_H
+ gl_TIME_R
+ if test $HAVE_LOCALTIME_R = 0 || test $REPLACE_LOCALTIME_R = 1; then
+ AC_LIBOBJ([time_r])
+ gl_PREREQ_TIME_R
+ fi
+ gl_TIME_MODULE_INDICATOR([time_r])
gl_UNISTD_H
gl_UNISTD_SAFER
gl_FUNC_UNSETENV
@@ -934,6 +941,7 @@
lib/tempname.c
lib/tempname.h
lib/time.in.h
+ lib/time_r.c
lib/unistd--.h
lib/unistd-safer.h
lib/unistd.c
@@ -1075,6 +1083,7 @@
m4/tempname.m4
m4/threadlib.m4
m4/time_h.m4
+ m4/time_r.m4
m4/unistd-safer.m4
m4/unistd_h.m4
m4/warn-on-use.m4
new file mode 100644
@@ -0,0 +1,58 @@
+dnl Reentrant time functions: localtime_r, gmtime_r.
+
+dnl Copyright (C) 2003, 2006-2016 Free Software Foundation, Inc.
+dnl This file is free software; the Free Software Foundation
+dnl gives unlimited permission to copy and/or distribute it,
+dnl with or without modifications, as long as this notice is preserved.
+
+dnl Written by Paul Eggert.
+
+AC_DEFUN([gl_TIME_R],
+[
+ dnl Persuade glibc and Solaris <time.h> to declare localtime_r.
+ AC_REQUIRE([gl_USE_SYSTEM_EXTENSIONS])
+
+ AC_REQUIRE([gl_HEADER_TIME_H_DEFAULTS])
+ AC_REQUIRE([AC_C_RESTRICT])
+
+ dnl Some systems don't declare localtime_r() and gmtime_r() if _REENTRANT is
+ dnl not defined.
+ AC_CHECK_DECLS([localtime_r], [], [], [[#include <time.h>]])
+ if test $ac_cv_have_decl_localtime_r = no; then
+ HAVE_DECL_LOCALTIME_R=0
+ fi
+
+ AC_CHECK_FUNCS_ONCE([localtime_r])
+ if test $ac_cv_func_localtime_r = yes; then
+ HAVE_LOCALTIME_R=1
+ AC_CACHE_CHECK([whether localtime_r is compatible with its POSIX signature],
+ [gl_cv_time_r_posix],
+ [AC_COMPILE_IFELSE(
+ [AC_LANG_PROGRAM(
+ [[#include <time.h>]],
+ [[/* We don't need to append 'restrict's to the argument types,
+ even though the POSIX signature has the 'restrict's,
+ since C99 says they can't affect type compatibility. */
+ struct tm * (*ptr) (time_t const *, struct tm *) = localtime_r;
+ if (ptr) return 0;
+ /* Check the return type is a pointer.
+ On HP-UX 10 it is 'int'. */
+ *localtime_r (0, 0);]])
+ ],
+ [gl_cv_time_r_posix=yes],
+ [gl_cv_time_r_posix=no])
+ ])
+ if test $gl_cv_time_r_posix = yes; then
+ REPLACE_LOCALTIME_R=0
+ else
+ REPLACE_LOCALTIME_R=1
+ fi
+ else
+ HAVE_LOCALTIME_R=0
+ fi
+])
+
+# Prerequisites of lib/time_r.c.
+AC_DEFUN([gl_PREREQ_TIME_R], [
+ :
+])
new file mode 100644
@@ -0,0 +1,44 @@
+/* Reentrant time functions like localtime_r.
+
+ Copyright (C) 2003, 2006-2007, 2010-2016 Free Software Foundation, Inc.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3, or (at your option)
+ any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License along
+ with this program; if not, see <http://www.gnu.org/licenses/>. */
+
+/* Written by Paul Eggert. */
+
+#include <config.h>
+
+#include <time.h>
+
+static struct tm *
+copy_tm_result (struct tm *dest, struct tm const *src)
+{
+ if (! src)
+ return 0;
+ *dest = *src;
+ return dest;
+}
+
+
+struct tm *
+gmtime_r (time_t const * restrict t, struct tm * restrict tp)
+{
+ return copy_tm_result (tp, gmtime (t));
+}
+
+struct tm *
+localtime_r (time_t const * restrict t, struct tm * restrict tp)
+{
+ return copy_tm_result (tp, localtime (t));
+}
@@ -59,6 +59,7 @@
strstr \
strtok_r \
sys_stat \
+ time_r \
unistd \
unsetenv \
update-copyright \