* [review] Use ctime_r and localtime_r if available
2019-10-31 21:06 [review] Use ctime_r and localtime_r if available Christian Biesinger (Code Review)
@ 2019-10-31 21:15 ` Christian Biesinger (Code Review)
2019-11-03 2:54 ` Christian Biesinger (Code Review)
` (12 subsequent siblings)
13 siblings, 0 replies; 20+ messages in thread
From: Christian Biesinger (Code Review) @ 2019-10-31 21:15 UTC (permalink / raw)
To: Christian Biesinger, gdb-patches
Christian Biesinger has posted comments on this change.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/475
......................................................................
Patch Set 1:
OK, this fails on Solaris:
https://gdb-buildbot.osci.io/#builders/11/builds/1069
I am not planning to work further on this. Someone more familiar with autoconf and/or Solaris can pick this up.
--
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I329bbdc39d5b576f51859ba00f1617e024c30cbd
Gerrit-Change-Number: 475
Gerrit-PatchSet: 1
Gerrit-Owner: Christian Biesinger <cbiesinger@google.com>
Gerrit-Reviewer: Christian Biesinger <cbiesinger@google.com>
Gerrit-Comment-Date: Thu, 31 Oct 2019 21:15:50 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
^ permalink raw reply [flat|nested] 20+ messages in thread
* [review] Use ctime_r and localtime_r if available
2019-10-31 21:06 [review] Use ctime_r and localtime_r if available Christian Biesinger (Code Review)
2019-10-31 21:15 ` Christian Biesinger (Code Review)
@ 2019-11-03 2:54 ` Christian Biesinger (Code Review)
2019-11-03 2:54 ` [review v2] " Christian Biesinger (Code Review)
` (11 subsequent siblings)
13 siblings, 0 replies; 20+ messages in thread
From: Christian Biesinger (Code Review) @ 2019-11-03 2:54 UTC (permalink / raw)
To: Christian Biesinger, gdb-patches
Christian Biesinger has posted comments on this change.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/475
......................................................................
Patch Set 1:
I must've tested this incorrectly before; the previous version was overly complicated. Will upload a new version in a second that works fine.
--
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I329bbdc39d5b576f51859ba00f1617e024c30cbd
Gerrit-Change-Number: 475
Gerrit-PatchSet: 1
Gerrit-Owner: Christian Biesinger <cbiesinger@google.com>
Gerrit-Reviewer: Christian Biesinger <cbiesinger@google.com>
Gerrit-Comment-Date: Sun, 03 Nov 2019 02:53:59 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
^ permalink raw reply [flat|nested] 20+ messages in thread
* [review v2] Use ctime_r and localtime_r if available
2019-10-31 21:06 [review] Use ctime_r and localtime_r if available Christian Biesinger (Code Review)
2019-10-31 21:15 ` Christian Biesinger (Code Review)
2019-11-03 2:54 ` Christian Biesinger (Code Review)
@ 2019-11-03 2:54 ` Christian Biesinger (Code Review)
2019-11-03 7:20 ` Andreas Schwab
2019-11-03 20:04 ` [review v3] " Christian Biesinger (Code Review)
` (10 subsequent siblings)
13 siblings, 1 reply; 20+ messages in thread
From: Christian Biesinger (Code Review) @ 2019-11-03 2:54 UTC (permalink / raw)
To: Christian Biesinger, gdb-patches
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/475
......................................................................
Use ctime_r and localtime_r if available
To make these calls threadsafe.
gdb/ChangeLog:
2019-10-31 Christian Biesinger <cbiesinger@google.com>
* config.in: Regenerate.
* configure: Regenerate.
* gdbsupport/common.m4: Check for ctime_r.
* configure.ac: Check for localtime_r.
* maint.c (scoped_command_stats::print_time): Use localtime_r
when available instead of localtime.
* nat/linux-osdata.c (time_from_time_t): Use ctime_r if available
instead of ctime.
gdb/gdbserver/ChangeLog:
2019-10-31 Christian Biesinger <cbiesinger@google.com>
* config.in: Regenerate.
* configure: Regenerate.
Change-Id: I329bbdc39d5b576f51859ba00f1617e024c30cbd
---
M gdb/config.in
M gdb/configure
M gdb/configure.ac
M gdb/gdbserver/config.in
M gdb/gdbserver/configure
M gdb/gdbsupport/common.m4
M gdb/maint.c
M gdb/nat/linux-osdata.c
8 files changed, 26 insertions(+), 6 deletions(-)
diff --git a/gdb/config.in b/gdb/config.in
index 5a21fca..9b29196 100644
--- a/gdb/config.in
+++ b/gdb/config.in
@@ -78,6 +78,9 @@
/* Define to 1 if you have the `btowc' function. */
#undef HAVE_BTOWC
+/* Define to 1 if you have the `ctime_r' function. */
+#undef HAVE_CTIME_R
+
/* Define to 1 if you have the <cursesX.h> header file. */
#undef HAVE_CURSESX_H
@@ -261,6 +264,9 @@
/* Define to 1 if you have the <locale.h> header file. */
#undef HAVE_LOCALE_H
+/* Define to 1 if you have the `localtime_r' function. */
+#undef HAVE_LOCALTIME_R
+
/* Define to 1 if the compiler supports long double. */
#undef HAVE_LONG_DOUBLE
diff --git a/gdb/configure b/gdb/configure
index 512f016..f5444df 100755
--- a/gdb/configure
+++ b/gdb/configure
@@ -13073,7 +13073,7 @@
sigaction sigsetmask socketpair \
ttrace wborder wresize setlocale iconvlist libiconvlist btowc \
setrlimit getrlimit posix_madvise waitpid \
- ptrace64 sigaltstack setns use_default_colors
+ ptrace64 sigaltstack setns use_default_colors localtime_r
do :
as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh`
ac_fn_c_check_func "$LINENO" "$ac_func" "$as_ac_var"
@@ -13480,7 +13480,7 @@
for ac_func in fdwalk getrlimit pipe pipe2 socketpair sigaction \
- sigprocmask strerror_r
+ sigprocmask strerror_r ctime_r
do :
as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh`
ac_fn_c_check_func "$LINENO" "$ac_func" "$as_ac_var"
diff --git a/gdb/configure.ac b/gdb/configure.ac
index 354bb7b..3d6f763 100644
--- a/gdb/configure.ac
+++ b/gdb/configure.ac
@@ -1318,7 +1318,7 @@
sigaction sigsetmask socketpair \
ttrace wborder wresize setlocale iconvlist libiconvlist btowc \
setrlimit getrlimit posix_madvise waitpid \
- ptrace64 sigaltstack setns use_default_colors])
+ ptrace64 sigaltstack setns use_default_colors localtime_r])
AM_LANGINFO_CODESET
GDB_AC_COMMON
diff --git a/gdb/gdbserver/config.in b/gdb/gdbserver/config.in
index 2984281..547a942 100644
--- a/gdb/gdbserver/config.in
+++ b/gdb/gdbserver/config.in
@@ -21,6 +21,9 @@
/* Define to 1 if you have the <arpa/inet.h> header file. */
#undef HAVE_ARPA_INET_H
+/* Define to 1 if you have the `ctime_r' function. */
+#undef HAVE_CTIME_R
+
/* define if the compiler supports basic C++11 syntax */
#undef HAVE_CXX11
diff --git a/gdb/gdbserver/configure b/gdb/gdbserver/configure
index 3f1f1c1..352836f 100755
--- a/gdb/gdbserver/configure
+++ b/gdb/gdbserver/configure
@@ -6822,7 +6822,7 @@
for ac_func in fdwalk getrlimit pipe pipe2 socketpair sigaction \
- sigprocmask strerror_r
+ sigprocmask strerror_r ctime_r
do :
as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh`
ac_fn_c_check_func "$LINENO" "$ac_func" "$as_ac_var"
diff --git a/gdb/gdbsupport/common.m4 b/gdb/gdbsupport/common.m4
index 2e44cf4..81f3fec 100644
--- a/gdb/gdbsupport/common.m4
+++ b/gdb/gdbsupport/common.m4
@@ -33,7 +33,7 @@
dlfcn.h)
AC_CHECK_FUNCS([fdwalk getrlimit pipe pipe2 socketpair sigaction \
- sigprocmask strerror_r])
+ sigprocmask strerror_r ctime_r])
AC_CHECK_DECLS([strerror, strstr])
diff --git a/gdb/maint.c b/gdb/maint.c
index ec9f4ab..36ab716 100644
--- a/gdb/maint.c
+++ b/gdb/maint.c
@@ -1039,7 +1039,12 @@
auto millis = ticks % 1000;
std::time_t as_time = system_clock::to_time_t (now);
+#ifdef HAVE_LOCALTIME_R
+ struct tm buf;
+ struct tm *tm = localtime_r (&as_time, &buf);
+#else
struct tm *tm = localtime (&as_time);
+#endif
char out[100];
strftime (out, sizeof (out), "%F %H:%M:%S", tm);
diff --git a/gdb/nat/linux-osdata.c b/gdb/nat/linux-osdata.c
index 67f9f3a..e0bad81 100644
--- a/gdb/nat/linux-osdata.c
+++ b/gdb/nat/linux-osdata.c
@@ -912,7 +912,13 @@
{
time_t t = (time_t) seconds;
- strncpy (time, ctime (&t), maxlen);
+ char buf[30];
+#ifdef HAVE_CTIME_R
+ const char *time_str = ctime_r (&t, buf);
+#else
+ const char *time_str = ctime (&t);
+#endif
+ strncpy (time, time_str, maxlen);
time[maxlen - 1] = '\0';
}
}
--
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I329bbdc39d5b576f51859ba00f1617e024c30cbd
Gerrit-Change-Number: 475
Gerrit-PatchSet: 2
Gerrit-Owner: Christian Biesinger <cbiesinger@google.com>
Gerrit-Reviewer: Christian Biesinger <cbiesinger@google.com>
Gerrit-MessageType: newpatchset
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [review v2] Use ctime_r and localtime_r if available
2019-11-03 2:54 ` [review v2] " Christian Biesinger (Code Review)
@ 2019-11-03 7:20 ` Andreas Schwab
2019-11-03 20:09 ` Christian Biesinger via gdb-patches
0 siblings, 1 reply; 20+ messages in thread
From: Andreas Schwab @ 2019-11-03 7:20 UTC (permalink / raw)
To: Christian Biesinger (Code Review); +Cc: Christian Biesinger, gdb-patches
On Nov 02 2019, Christian Biesinger (Code Review) wrote:
> diff --git a/gdb/nat/linux-osdata.c b/gdb/nat/linux-osdata.c
> index 67f9f3a..e0bad81 100644
> --- a/gdb/nat/linux-osdata.c
> +++ b/gdb/nat/linux-osdata.c
> @@ -912,7 +912,13 @@
> {
> time_t t = (time_t) seconds;
>
> - strncpy (time, ctime (&t), maxlen);
> + char buf[30];
> +#ifdef HAVE_CTIME_R
> + const char *time_str = ctime_r (&t, buf);
> +#else
> + const char *time_str = ctime (&t);
> +#endif
buf is unused if !HAVE_CTIME_R.
Note that both ctime and ctime_r are obsolescent and should be replaced
by strftime. gdb currently doesn't setlocale LC_TIME, but if it does it
would make the use of these functions undefined.
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [review v2] Use ctime_r and localtime_r if available
2019-11-03 7:20 ` Andreas Schwab
@ 2019-11-03 20:09 ` Christian Biesinger via gdb-patches
0 siblings, 0 replies; 20+ messages in thread
From: Christian Biesinger via gdb-patches @ 2019-11-03 20:09 UTC (permalink / raw)
To: Andreas Schwab; +Cc: Christian Biesinger (Code Review), gdb-patches
On Sun, Nov 3, 2019 at 2:20 AM Andreas Schwab <schwab@linux-m68k.org> wrote:
>
> On Nov 02 2019, Christian Biesinger (Code Review) wrote:
>
> > diff --git a/gdb/nat/linux-osdata.c b/gdb/nat/linux-osdata.c
> > index 67f9f3a..e0bad81 100644
> > --- a/gdb/nat/linux-osdata.c
> > +++ b/gdb/nat/linux-osdata.c
> > @@ -912,7 +912,13 @@
> > {
> > time_t t = (time_t) seconds;
> >
> > - strncpy (time, ctime (&t), maxlen);
> > + char buf[30];
> > +#ifdef HAVE_CTIME_R
> > + const char *time_str = ctime_r (&t, buf);
> > +#else
> > + const char *time_str = ctime (&t);
> > +#endif
>
> buf is unused if !HAVE_CTIME_R.
Thanks, fixed.
> Note that both ctime and ctime_r are obsolescent and should be replaced
> by strftime. gdb currently doesn't setlocale LC_TIME, but if it does it
> would make the use of these functions undefined.
I read through the manpage for ctime/ctime_r and I don't see any note
that the behavior is undefined when setlocale is called, or that it is
obsolete. For completeness, I also looked at the Solaris manpage and
it does not say that either. Instead, it will use a locale-independent
format, which seems desirable here since it is serialized into XML.
Christian
^ permalink raw reply [flat|nested] 20+ messages in thread
* [review v3] Use ctime_r and localtime_r if available
2019-10-31 21:06 [review] Use ctime_r and localtime_r if available Christian Biesinger (Code Review)
` (2 preceding siblings ...)
2019-11-03 2:54 ` [review v2] " Christian Biesinger (Code Review)
@ 2019-11-03 20:04 ` Christian Biesinger (Code Review)
2019-11-06 20:30 ` Christian Biesinger (Code Review)
` (9 subsequent siblings)
13 siblings, 0 replies; 20+ messages in thread
From: Christian Biesinger (Code Review) @ 2019-11-03 20:04 UTC (permalink / raw)
To: Christian Biesinger, gdb-patches
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/475
......................................................................
Use ctime_r and localtime_r if available
To make these calls threadsafe.
gdb/ChangeLog:
2019-10-31 Christian Biesinger <cbiesinger@google.com>
* config.in: Regenerate.
* configure: Regenerate.
* gdbsupport/common.m4: Check for ctime_r.
* configure.ac: Check for localtime_r.
* maint.c (scoped_command_stats::print_time): Use localtime_r
when available instead of localtime.
* nat/linux-osdata.c (time_from_time_t): Use ctime_r if available
instead of ctime.
gdb/gdbserver/ChangeLog:
2019-10-31 Christian Biesinger <cbiesinger@google.com>
* config.in: Regenerate.
* configure: Regenerate.
Change-Id: I329bbdc39d5b576f51859ba00f1617e024c30cbd
---
M gdb/config.in
M gdb/configure
M gdb/configure.ac
M gdb/gdbserver/config.in
M gdb/gdbserver/configure
M gdb/gdbsupport/common.m4
M gdb/maint.c
M gdb/nat/linux-osdata.c
8 files changed, 26 insertions(+), 6 deletions(-)
diff --git a/gdb/config.in b/gdb/config.in
index 5a21fca..9b29196 100644
--- a/gdb/config.in
+++ b/gdb/config.in
@@ -78,6 +78,9 @@
/* Define to 1 if you have the `btowc' function. */
#undef HAVE_BTOWC
+/* Define to 1 if you have the `ctime_r' function. */
+#undef HAVE_CTIME_R
+
/* Define to 1 if you have the <cursesX.h> header file. */
#undef HAVE_CURSESX_H
@@ -261,6 +264,9 @@
/* Define to 1 if you have the <locale.h> header file. */
#undef HAVE_LOCALE_H
+/* Define to 1 if you have the `localtime_r' function. */
+#undef HAVE_LOCALTIME_R
+
/* Define to 1 if the compiler supports long double. */
#undef HAVE_LONG_DOUBLE
diff --git a/gdb/configure b/gdb/configure
index 512f016..f5444df 100755
--- a/gdb/configure
+++ b/gdb/configure
@@ -13073,7 +13073,7 @@
sigaction sigsetmask socketpair \
ttrace wborder wresize setlocale iconvlist libiconvlist btowc \
setrlimit getrlimit posix_madvise waitpid \
- ptrace64 sigaltstack setns use_default_colors
+ ptrace64 sigaltstack setns use_default_colors localtime_r
do :
as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh`
ac_fn_c_check_func "$LINENO" "$ac_func" "$as_ac_var"
@@ -13480,7 +13480,7 @@
for ac_func in fdwalk getrlimit pipe pipe2 socketpair sigaction \
- sigprocmask strerror_r
+ sigprocmask strerror_r ctime_r
do :
as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh`
ac_fn_c_check_func "$LINENO" "$ac_func" "$as_ac_var"
diff --git a/gdb/configure.ac b/gdb/configure.ac
index 354bb7b..3d6f763 100644
--- a/gdb/configure.ac
+++ b/gdb/configure.ac
@@ -1318,7 +1318,7 @@
sigaction sigsetmask socketpair \
ttrace wborder wresize setlocale iconvlist libiconvlist btowc \
setrlimit getrlimit posix_madvise waitpid \
- ptrace64 sigaltstack setns use_default_colors])
+ ptrace64 sigaltstack setns use_default_colors localtime_r])
AM_LANGINFO_CODESET
GDB_AC_COMMON
diff --git a/gdb/gdbserver/config.in b/gdb/gdbserver/config.in
index 2984281..547a942 100644
--- a/gdb/gdbserver/config.in
+++ b/gdb/gdbserver/config.in
@@ -21,6 +21,9 @@
/* Define to 1 if you have the <arpa/inet.h> header file. */
#undef HAVE_ARPA_INET_H
+/* Define to 1 if you have the `ctime_r' function. */
+#undef HAVE_CTIME_R
+
/* define if the compiler supports basic C++11 syntax */
#undef HAVE_CXX11
diff --git a/gdb/gdbserver/configure b/gdb/gdbserver/configure
index 3f1f1c1..352836f 100755
--- a/gdb/gdbserver/configure
+++ b/gdb/gdbserver/configure
@@ -6822,7 +6822,7 @@
for ac_func in fdwalk getrlimit pipe pipe2 socketpair sigaction \
- sigprocmask strerror_r
+ sigprocmask strerror_r ctime_r
do :
as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh`
ac_fn_c_check_func "$LINENO" "$ac_func" "$as_ac_var"
diff --git a/gdb/gdbsupport/common.m4 b/gdb/gdbsupport/common.m4
index 2e44cf4..81f3fec 100644
--- a/gdb/gdbsupport/common.m4
+++ b/gdb/gdbsupport/common.m4
@@ -33,7 +33,7 @@
dlfcn.h)
AC_CHECK_FUNCS([fdwalk getrlimit pipe pipe2 socketpair sigaction \
- sigprocmask strerror_r])
+ sigprocmask strerror_r ctime_r])
AC_CHECK_DECLS([strerror, strstr])
diff --git a/gdb/maint.c b/gdb/maint.c
index ec9f4ab..36ab716 100644
--- a/gdb/maint.c
+++ b/gdb/maint.c
@@ -1039,7 +1039,12 @@
auto millis = ticks % 1000;
std::time_t as_time = system_clock::to_time_t (now);
+#ifdef HAVE_LOCALTIME_R
+ struct tm buf;
+ struct tm *tm = localtime_r (&as_time, &buf);
+#else
struct tm *tm = localtime (&as_time);
+#endif
char out[100];
strftime (out, sizeof (out), "%F %H:%M:%S", tm);
diff --git a/gdb/nat/linux-osdata.c b/gdb/nat/linux-osdata.c
index 67f9f3a..9d6f86a 100644
--- a/gdb/nat/linux-osdata.c
+++ b/gdb/nat/linux-osdata.c
@@ -912,7 +912,13 @@
{
time_t t = (time_t) seconds;
- strncpy (time, ctime (&t), maxlen);
+#ifdef HAVE_CTIME_R
+ char buf[30];
+ const char *time_str = ctime_r (&t, buf);
+#else
+ const char *time_str = ctime (&t);
+#endif
+ strncpy (time, time_str, maxlen);
time[maxlen - 1] = '\0';
}
}
--
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I329bbdc39d5b576f51859ba00f1617e024c30cbd
Gerrit-Change-Number: 475
Gerrit-PatchSet: 3
Gerrit-Owner: Christian Biesinger <cbiesinger@google.com>
Gerrit-Reviewer: Christian Biesinger <cbiesinger@google.com>
Gerrit-MessageType: newpatchset
^ permalink raw reply [flat|nested] 20+ messages in thread
* [review v3] Use ctime_r and localtime_r if available
2019-10-31 21:06 [review] Use ctime_r and localtime_r if available Christian Biesinger (Code Review)
` (3 preceding siblings ...)
2019-11-03 20:04 ` [review v3] " Christian Biesinger (Code Review)
@ 2019-11-06 20:30 ` Christian Biesinger (Code Review)
2019-11-08 14:09 ` Pedro Alves
2019-11-10 7:38 ` Kevin Buettner (Code Review)
` (8 subsequent siblings)
13 siblings, 1 reply; 20+ messages in thread
From: Christian Biesinger (Code Review) @ 2019-11-06 20:30 UTC (permalink / raw)
To: Christian Biesinger, gdb-patches
Christian Biesinger has posted comments on this change.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/475
......................................................................
Patch Set 3:
I looked at gnulib's time_r module but it turns out that its localtime_r implementation is not in any way threadsafe.
--
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I329bbdc39d5b576f51859ba00f1617e024c30cbd
Gerrit-Change-Number: 475
Gerrit-PatchSet: 3
Gerrit-Owner: Christian Biesinger <cbiesinger@google.com>
Gerrit-Reviewer: Christian Biesinger <cbiesinger@google.com>
Gerrit-Comment-Date: Wed, 06 Nov 2019 20:30:40 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [review v3] Use ctime_r and localtime_r if available
2019-11-06 20:30 ` Christian Biesinger (Code Review)
@ 2019-11-08 14:09 ` Pedro Alves
2019-11-08 17:11 ` Christian Biesinger via gdb-patches
0 siblings, 1 reply; 20+ messages in thread
From: Pedro Alves @ 2019-11-08 14:09 UTC (permalink / raw)
To: gnutoolchain-gerrit, Christian Biesinger, gdb-patches
On 11/6/19 8:30 PM, Christian Biesinger (Code Review) wrote:
> Patch Set 3:
>
> I looked at gnulib's time_r module but it turns out that its localtime_r implementation is not in any way threadsafe.
It does seemingly look that way:
struct tm *
localtime_r (time_t const * restrict t, struct tm * restrict tp)
{
return copy_tm_result (tp, localtime (t));
}
But that doesn't seem worse than the #ifdef fallback that you're leaving
in place. I'd argue that it'd be better to use the gnulib version.
But read on, please. Looking at:
https://www.gnu.org/software/gnulib/manual/html_node/localtime_005fr.html
Of the problems that are relevant on our supported hosts, we see:
Portability problems fixed by Gnulib:
This function is missing on some platforms: mingw, MSVC 14.
And, note that localtime, like other C functions that use global
state, are thread safe on Windows, because the C run time stores
the global buffers in thread local storage. So the gnulib
implementation ends up being thread safe there, even though
it doesn't look like it is.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [review v3] Use ctime_r and localtime_r if available
2019-11-08 14:09 ` Pedro Alves
@ 2019-11-08 17:11 ` Christian Biesinger via gdb-patches
2019-11-09 20:18 ` Christian Biesinger via gdb-patches
0 siblings, 1 reply; 20+ messages in thread
From: Christian Biesinger via gdb-patches @ 2019-11-08 17:11 UTC (permalink / raw)
To: Pedro Alves; +Cc: gnutoolchain-gerrit, gdb-patches
On Fri, Nov 8, 2019 at 8:08 AM Pedro Alves <palves@redhat.com> wrote:
> And, note that localtime, like other C functions that use global
> state, are thread safe on Windows, because the C run time stores
> the global buffers in thread local storage. So the gnulib
> implementation ends up being thread safe there, even though
> it doesn't look like it is.
OK, I'll change this to use the time_r module.
I'll make it depend on
https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/514 since that
one also imports a new module.
Christian
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [review v3] Use ctime_r and localtime_r if available
2019-11-08 17:11 ` Christian Biesinger via gdb-patches
@ 2019-11-09 20:18 ` Christian Biesinger via gdb-patches
0 siblings, 0 replies; 20+ messages in thread
From: Christian Biesinger via gdb-patches @ 2019-11-09 20:18 UTC (permalink / raw)
To: Pedro Alves; +Cc: gnutoolchain-gerrit, gdb-patches
On Fri, Nov 8, 2019 at 11:10 AM Christian Biesinger
<cbiesinger@google.com> wrote:
>
> On Fri, Nov 8, 2019 at 8:08 AM Pedro Alves <palves@redhat.com> wrote:
> > And, note that localtime, like other C functions that use global
> > state, are thread safe on Windows, because the C run time stores
> > the global buffers in thread local storage. So the gnulib
> > implementation ends up being thread safe there, even though
> > it doesn't look like it is.
>
> OK, I'll change this to use the time_r module.
>
> I'll make it depend on
> https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/514 since that
> one also imports a new module.
So I tried gnulib's time_r module but it failed to compile on the
mingw buildbot:
https://gdb-buildbot.osci.io/#builders/23/builds/934
Any ideas?
Christian
^ permalink raw reply [flat|nested] 20+ messages in thread
* [review v3] Use ctime_r and localtime_r if available
2019-10-31 21:06 [review] Use ctime_r and localtime_r if available Christian Biesinger (Code Review)
` (4 preceding siblings ...)
2019-11-06 20:30 ` Christian Biesinger (Code Review)
@ 2019-11-10 7:38 ` Kevin Buettner (Code Review)
2019-11-10 7:45 ` Kevin Buettner (Code Review)
` (7 subsequent siblings)
13 siblings, 0 replies; 20+ messages in thread
From: Kevin Buettner (Code Review) @ 2019-11-10 7:38 UTC (permalink / raw)
To: Christian Biesinger, gdb-patches
Kevin Buettner has posted comments on this change.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/475
......................................................................
Patch Set 3: Code-Review+2
--
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I329bbdc39d5b576f51859ba00f1617e024c30cbd
Gerrit-Change-Number: 475
Gerrit-PatchSet: 3
Gerrit-Owner: Christian Biesinger <cbiesinger@google.com>
Gerrit-Reviewer: Christian Biesinger <cbiesinger@google.com>
Gerrit-Reviewer: Kevin Buettner <kevinb@redhat.com>
Gerrit-Comment-Date: Sun, 10 Nov 2019 07:38:34 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
^ permalink raw reply [flat|nested] 20+ messages in thread
* [review v3] Use ctime_r and localtime_r if available
2019-10-31 21:06 [review] Use ctime_r and localtime_r if available Christian Biesinger (Code Review)
` (5 preceding siblings ...)
2019-11-10 7:38 ` Kevin Buettner (Code Review)
@ 2019-11-10 7:45 ` Kevin Buettner (Code Review)
2019-11-11 22:22 ` [review v4] " Christian Biesinger (Code Review)
` (6 subsequent siblings)
13 siblings, 0 replies; 20+ messages in thread
From: Kevin Buettner (Code Review) @ 2019-11-10 7:45 UTC (permalink / raw)
To: Christian Biesinger, gdb-patches
Kevin Buettner has posted comments on this change.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/475
......................................................................
Patch Set 3:
(1 comment)
> Patch Set 3: Code-Review+2
Only one small nit in gdb/nat/linux-osdata.c - I suggest adding a comment. Aside from that, it looks good to me.
| --- gdb/nat/linux-osdata.c
| +++ gdb/nat/linux-osdata.c
| @@ -908,12 +908,18 @@ time_from_time_t (char *time, int maxlen, TIME_T seconds)
| {
| if (!seconds)
| time[0] = '\0';
| else
| {
| time_t t = (time_t) seconds;
|
| - strncpy (time, ctime (&t), maxlen);
| +#ifdef HAVE_CTIME_R
| + char buf[30];
PS3, Line 916:
Perhaps add a comment here stating that the ctime_r man page states
that the user supplied buffer needs to be 26 bytes or larger?
| + const char *time_str = ctime_r (&t, buf);
| +#else
| + const char *time_str = ctime (&t);
| +#endif
| + strncpy (time, time_str, maxlen);
| time[maxlen - 1] = '\0';
| }
| }
|
--
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I329bbdc39d5b576f51859ba00f1617e024c30cbd
Gerrit-Change-Number: 475
Gerrit-PatchSet: 3
Gerrit-Owner: Christian Biesinger <cbiesinger@google.com>
Gerrit-Reviewer: Christian Biesinger <cbiesinger@google.com>
Gerrit-Reviewer: Kevin Buettner <kevinb@redhat.com>
Gerrit-Comment-Date: Sun, 10 Nov 2019 07:45:29 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
^ permalink raw reply [flat|nested] 20+ messages in thread
* [review v4] Use ctime_r and localtime_r if available
2019-10-31 21:06 [review] Use ctime_r and localtime_r if available Christian Biesinger (Code Review)
` (6 preceding siblings ...)
2019-11-10 7:45 ` Kevin Buettner (Code Review)
@ 2019-11-11 22:22 ` Christian Biesinger (Code Review)
2019-11-11 22:27 ` [review v5] Use ctime_r and localtime_r for threadsafety Christian Biesinger (Code Review)
` (5 subsequent siblings)
13 siblings, 0 replies; 20+ messages in thread
From: Christian Biesinger (Code Review) @ 2019-11-11 22:22 UTC (permalink / raw)
To: Christian Biesinger, Kevin Buettner, gdb-patches
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/475
......................................................................
Use ctime_r and localtime_r if available
To make these calls threadsafe.
gdb/ChangeLog:
2019-10-31 Christian Biesinger <cbiesinger@google.com>
* config.in: Regenerate.
* configure: Regenerate.
* gdbsupport/common.m4: Check for ctime_r.
* maint.c (scoped_command_stats::print_time): Use localtime_r
instead of localtime (provided through gnulib if necessary).
* nat/linux-osdata.c (time_from_time_t): Use ctime_r if available
instead of ctime.
gdb/gdbserver/ChangeLog:
2019-10-31 Christian Biesinger <cbiesinger@google.com>
* config.in: Regenerate.
* configure: Regenerate.
Change-Id: I329bbdc39d5b576f51859ba00f1617e024c30cbd
---
M gdb/config.in
M gdb/configure
M gdb/gdbserver/config.in
M gdb/gdbserver/configure
M gdb/gdbsupport/common.m4
M gdb/maint.c
M gdb/nat/linux-osdata.c
7 files changed, 19 insertions(+), 6 deletions(-)
diff --git a/gdb/config.in b/gdb/config.in
index fc05f15..19b18d1 100644
--- a/gdb/config.in
+++ b/gdb/config.in
@@ -78,6 +78,9 @@
/* Define to 1 if you have the `btowc' function. */
#undef HAVE_BTOWC
+/* Define to 1 if you have the `ctime_r' function. */
+#undef HAVE_CTIME_R
+
/* Define to 1 if you have the <cursesX.h> header file. */
#undef HAVE_CURSESX_H
diff --git a/gdb/configure b/gdb/configure
index e805903..78c0f34 100755
--- a/gdb/configure
+++ b/gdb/configure
@@ -13480,7 +13480,7 @@
for ac_func in fdwalk getrlimit pipe pipe2 socketpair sigaction \
- sigprocmask
+ sigprocmask ctime_r
do :
as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh`
ac_fn_c_check_func "$LINENO" "$ac_func" "$as_ac_var"
diff --git a/gdb/gdbserver/config.in b/gdb/gdbserver/config.in
index 0bce18d..efa2ea0 100644
--- a/gdb/gdbserver/config.in
+++ b/gdb/gdbserver/config.in
@@ -21,6 +21,9 @@
/* Define to 1 if you have the <arpa/inet.h> header file. */
#undef HAVE_ARPA_INET_H
+/* Define to 1 if you have the `ctime_r' function. */
+#undef HAVE_CTIME_R
+
/* define if the compiler supports basic C++11 syntax */
#undef HAVE_CXX11
diff --git a/gdb/gdbserver/configure b/gdb/gdbserver/configure
index e513fc5..0414aa7 100755
--- a/gdb/gdbserver/configure
+++ b/gdb/gdbserver/configure
@@ -6822,7 +6822,7 @@
for ac_func in fdwalk getrlimit pipe pipe2 socketpair sigaction \
- sigprocmask
+ sigprocmask ctime_r
do :
as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh`
ac_fn_c_check_func "$LINENO" "$ac_func" "$as_ac_var"
diff --git a/gdb/gdbsupport/common.m4 b/gdb/gdbsupport/common.m4
index 471d705..d86fc92 100644
--- a/gdb/gdbsupport/common.m4
+++ b/gdb/gdbsupport/common.m4
@@ -33,7 +33,7 @@
dlfcn.h)
AC_CHECK_FUNCS([fdwalk getrlimit pipe pipe2 socketpair sigaction \
- sigprocmask])
+ sigprocmask ctime_r])
AC_CHECK_DECLS([strerror, strstr])
diff --git a/gdb/maint.c b/gdb/maint.c
index ec9f4ab..a253584 100644
--- a/gdb/maint.c
+++ b/gdb/maint.c
@@ -1039,10 +1039,11 @@
auto millis = ticks % 1000;
std::time_t as_time = system_clock::to_time_t (now);
- struct tm *tm = localtime (&as_time);
+ struct tm tm;
+ localtime_r (&as_time, &tm);
char out[100];
- strftime (out, sizeof (out), "%F %H:%M:%S", tm);
+ strftime (out, sizeof (out), "%F %H:%M:%S", &tm);
printf_unfiltered ("%s.%03d - %s\n", out, (int) millis, msg);
}
diff --git a/gdb/nat/linux-osdata.c b/gdb/nat/linux-osdata.c
index 84357e2..cfa245a 100644
--- a/gdb/nat/linux-osdata.c
+++ b/gdb/nat/linux-osdata.c
@@ -913,7 +913,13 @@
{
time_t t = (time_t) seconds;
- strncpy (time, ctime (&t), maxlen);
+#ifdef HAVE_CTIME_R
+ char buf[30];
+ const char *time_str = ctime_r (&t, buf);
+#else
+ const char *time_str = ctime (&t);
+#endif
+ strncpy (time, time_str, maxlen);
time[maxlen - 1] = '\0';
}
}
--
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I329bbdc39d5b576f51859ba00f1617e024c30cbd
Gerrit-Change-Number: 475
Gerrit-PatchSet: 4
Gerrit-Owner: Christian Biesinger <cbiesinger@google.com>
Gerrit-Reviewer: Christian Biesinger <cbiesinger@google.com>
Gerrit-Reviewer: Kevin Buettner <kevinb@redhat.com>
Gerrit-MessageType: newpatchset
^ permalink raw reply [flat|nested] 20+ messages in thread
* [review v5] Use ctime_r and localtime_r for threadsafety
2019-10-31 21:06 [review] Use ctime_r and localtime_r if available Christian Biesinger (Code Review)
` (7 preceding siblings ...)
2019-11-11 22:22 ` [review v4] " Christian Biesinger (Code Review)
@ 2019-11-11 22:27 ` Christian Biesinger (Code Review)
2019-11-11 22:29 ` Christian Biesinger (Code Review)
` (4 subsequent siblings)
13 siblings, 0 replies; 20+ messages in thread
From: Christian Biesinger (Code Review) @ 2019-11-11 22:27 UTC (permalink / raw)
To: Christian Biesinger, Kevin Buettner, gdb-patches
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/475
......................................................................
Use ctime_r and localtime_r for threadsafety
To make these calls threadsafe. localtime_r is provided by gnulib if
necessary, and for ctime_r we can just use it because it is in a linux-
specific file.
gdb/ChangeLog:
2019-10-31 Christian Biesinger <cbiesinger@google.com>
* maint.c (scoped_command_stats::print_time): Use localtime_r
instead of localtime (provided through gnulib if necessary).
* nat/linux-osdata.c (time_from_time_t): Use ctime_r instead
of ctime.
Change-Id: I329bbdc39d5b576f51859ba00f1617e024c30cbd
---
M gdb/maint.c
M gdb/nat/linux-osdata.c
2 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/gdb/maint.c b/gdb/maint.c
index ec9f4ab..a253584 100644
--- a/gdb/maint.c
+++ b/gdb/maint.c
@@ -1039,10 +1039,11 @@
auto millis = ticks % 1000;
std::time_t as_time = system_clock::to_time_t (now);
- struct tm *tm = localtime (&as_time);
+ struct tm tm;
+ localtime_r (&as_time, &tm);
char out[100];
- strftime (out, sizeof (out), "%F %H:%M:%S", tm);
+ strftime (out, sizeof (out), "%F %H:%M:%S", &tm);
printf_unfiltered ("%s.%03d - %s\n", out, (int) millis, msg);
}
diff --git a/gdb/nat/linux-osdata.c b/gdb/nat/linux-osdata.c
index 84357e2..a0b43c6 100644
--- a/gdb/nat/linux-osdata.c
+++ b/gdb/nat/linux-osdata.c
@@ -913,7 +913,11 @@
{
time_t t = (time_t) seconds;
- strncpy (time, ctime (&t), maxlen);
+ /* Per the ctime_r manpage, this buffer needs to be at least 26
+ characters long. */
+ char buf[30];
+ const char *time_str = ctime_r (&t, buf);
+ strncpy (time, time_str, maxlen);
time[maxlen - 1] = '\0';
}
}
--
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I329bbdc39d5b576f51859ba00f1617e024c30cbd
Gerrit-Change-Number: 475
Gerrit-PatchSet: 5
Gerrit-Owner: Christian Biesinger <cbiesinger@google.com>
Gerrit-Reviewer: Christian Biesinger <cbiesinger@google.com>
Gerrit-Reviewer: Kevin Buettner <kevinb@redhat.com>
Gerrit-MessageType: newpatchset
^ permalink raw reply [flat|nested] 20+ messages in thread
* [review v5] Use ctime_r and localtime_r for threadsafety
2019-10-31 21:06 [review] Use ctime_r and localtime_r if available Christian Biesinger (Code Review)
` (8 preceding siblings ...)
2019-11-11 22:27 ` [review v5] Use ctime_r and localtime_r for threadsafety Christian Biesinger (Code Review)
@ 2019-11-11 22:29 ` Christian Biesinger (Code Review)
2019-11-12 20:21 ` Kevin Buettner (Code Review)
` (3 subsequent siblings)
13 siblings, 0 replies; 20+ messages in thread
From: Christian Biesinger (Code Review) @ 2019-11-11 22:29 UTC (permalink / raw)
To: Christian Biesinger, gdb-patches; +Cc: Kevin Buettner
Christian Biesinger has posted comments on this change.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/475
......................................................................
Patch Set 5:
Hi Kevin,
thanks for the review! I've simplified this now to rely on gnulib for localtime_r. I also realized that we can just unconditionally use ctime_r because it's a linux-specific like. Could you take another look (also on the dependencies)?
I did add a comment as you suggested.
--
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I329bbdc39d5b576f51859ba00f1617e024c30cbd
Gerrit-Change-Number: 475
Gerrit-PatchSet: 5
Gerrit-Owner: Christian Biesinger <cbiesinger@google.com>
Gerrit-Reviewer: Christian Biesinger <cbiesinger@google.com>
Gerrit-Reviewer: Kevin Buettner <kevinb@redhat.com>
Gerrit-Comment-Date: Mon, 11 Nov 2019 22:29:27 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
^ permalink raw reply [flat|nested] 20+ messages in thread
* [review v5] Use ctime_r and localtime_r for threadsafety
2019-10-31 21:06 [review] Use ctime_r and localtime_r if available Christian Biesinger (Code Review)
` (9 preceding siblings ...)
2019-11-11 22:29 ` Christian Biesinger (Code Review)
@ 2019-11-12 20:21 ` Kevin Buettner (Code Review)
2019-11-15 19:50 ` [review v6] " Christian Biesinger (Code Review)
` (2 subsequent siblings)
13 siblings, 0 replies; 20+ messages in thread
From: Kevin Buettner (Code Review) @ 2019-11-12 20:21 UTC (permalink / raw)
To: Christian Biesinger, gdb-patches
Kevin Buettner has posted comments on this change.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/475
......................................................................
Patch Set 5: Code-Review+2
--
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I329bbdc39d5b576f51859ba00f1617e024c30cbd
Gerrit-Change-Number: 475
Gerrit-PatchSet: 5
Gerrit-Owner: Christian Biesinger <cbiesinger@google.com>
Gerrit-Reviewer: Christian Biesinger <cbiesinger@google.com>
Gerrit-Reviewer: Kevin Buettner <kevinb@redhat.com>
Gerrit-Comment-Date: Tue, 12 Nov 2019 20:21:20 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
^ permalink raw reply [flat|nested] 20+ messages in thread
* [review v6] Use ctime_r and localtime_r for threadsafety
2019-10-31 21:06 [review] Use ctime_r and localtime_r if available Christian Biesinger (Code Review)
` (10 preceding siblings ...)
2019-11-12 20:21 ` Kevin Buettner (Code Review)
@ 2019-11-15 19:50 ` Christian Biesinger (Code Review)
2019-11-15 19:52 ` [pushed] " Sourceware to Gerrit sync (Code Review)
2019-11-16 22:07 ` [review v6] " Christian Biesinger (Code Review)
13 siblings, 0 replies; 20+ messages in thread
From: Christian Biesinger (Code Review) @ 2019-11-15 19:50 UTC (permalink / raw)
To: Christian Biesinger, Kevin Buettner, gdb-patches
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/475
......................................................................
Use ctime_r and localtime_r for threadsafety
To make these calls threadsafe. localtime_r is provided by gnulib if
necessary, and for ctime_r we can just use it because it is in a linux-
specific file.
gdb/ChangeLog:
2019-11-15 Christian Biesinger <cbiesinger@google.com>
* maint.c (scoped_command_stats::print_time): Use localtime_r
instead of localtime (provided through gnulib if necessary).
* nat/linux-osdata.c (time_from_time_t): Use ctime_r instead
of ctime.
Change-Id: I329bbdc39d5b576f51859ba00f1617e024c30cbd
---
M gdb/ChangeLog
M gdb/maint.c
M gdb/nat/linux-osdata.c
3 files changed, 15 insertions(+), 3 deletions(-)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index f31552b..f727aa4 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,12 @@
2019-11-15 Christian Biesinger <cbiesinger@google.com>
+ * maint.c (scoped_command_stats::print_time): Use localtime_r
+ instead of localtime (provided through gnulib if necessary).
+ * nat/linux-osdata.c (time_from_time_t): Use ctime_r instead
+ of ctime.
+
+2019-11-15 Christian Biesinger <cbiesinger@google.com>
+
* gdbsupport/common-defs.h: Include time.h before pathmax.h to
avoid compile errors.
diff --git a/gdb/maint.c b/gdb/maint.c
index ec9f4ab..a253584 100644
--- a/gdb/maint.c
+++ b/gdb/maint.c
@@ -1039,10 +1039,11 @@
auto millis = ticks % 1000;
std::time_t as_time = system_clock::to_time_t (now);
- struct tm *tm = localtime (&as_time);
+ struct tm tm;
+ localtime_r (&as_time, &tm);
char out[100];
- strftime (out, sizeof (out), "%F %H:%M:%S", tm);
+ strftime (out, sizeof (out), "%F %H:%M:%S", &tm);
printf_unfiltered ("%s.%03d - %s\n", out, (int) millis, msg);
}
diff --git a/gdb/nat/linux-osdata.c b/gdb/nat/linux-osdata.c
index ca6acd3..d82c062 100644
--- a/gdb/nat/linux-osdata.c
+++ b/gdb/nat/linux-osdata.c
@@ -916,7 +916,11 @@
{
time_t t = (time_t) seconds;
- strncpy (time, ctime (&t), maxlen);
+ /* Per the ctime_r manpage, this buffer needs to be at least 26
+ characters long. */
+ char buf[30];
+ const char *time_str = ctime_r (&t, buf);
+ strncpy (time, time_str, maxlen);
time[maxlen - 1] = '\0';
}
}
--
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I329bbdc39d5b576f51859ba00f1617e024c30cbd
Gerrit-Change-Number: 475
Gerrit-PatchSet: 6
Gerrit-Owner: Christian Biesinger <cbiesinger@google.com>
Gerrit-Reviewer: Christian Biesinger <cbiesinger@google.com>
Gerrit-Reviewer: Kevin Buettner <kevinb@redhat.com>
Gerrit-MessageType: newpatchset
^ permalink raw reply [flat|nested] 20+ messages in thread
* [pushed] Use ctime_r and localtime_r for threadsafety
2019-10-31 21:06 [review] Use ctime_r and localtime_r if available Christian Biesinger (Code Review)
` (11 preceding siblings ...)
2019-11-15 19:50 ` [review v6] " Christian Biesinger (Code Review)
@ 2019-11-15 19:52 ` Sourceware to Gerrit sync (Code Review)
2019-11-16 22:07 ` [review v6] " Christian Biesinger (Code Review)
13 siblings, 0 replies; 20+ messages in thread
From: Sourceware to Gerrit sync (Code Review) @ 2019-11-15 19:52 UTC (permalink / raw)
To: Christian Biesinger, gdb-patches; +Cc: Kevin Buettner
Sourceware to Gerrit sync has submitted this change.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/475
......................................................................
Use ctime_r and localtime_r for threadsafety
To make these calls threadsafe. localtime_r is provided by gnulib if
necessary, and for ctime_r we can just use it because it is in a linux-
specific file.
gdb/ChangeLog:
2019-11-15 Christian Biesinger <cbiesinger@google.com>
* maint.c (scoped_command_stats::print_time): Use localtime_r
instead of localtime (provided through gnulib if necessary).
* nat/linux-osdata.c (time_from_time_t): Use ctime_r instead
of ctime.
Change-Id: I329bbdc39d5b576f51859ba00f1617e024c30cbd
---
M gdb/ChangeLog
M gdb/maint.c
M gdb/nat/linux-osdata.c
3 files changed, 15 insertions(+), 3 deletions(-)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index f31552b..f727aa4 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,12 @@
2019-11-15 Christian Biesinger <cbiesinger@google.com>
+ * maint.c (scoped_command_stats::print_time): Use localtime_r
+ instead of localtime (provided through gnulib if necessary).
+ * nat/linux-osdata.c (time_from_time_t): Use ctime_r instead
+ of ctime.
+
+2019-11-15 Christian Biesinger <cbiesinger@google.com>
+
* gdbsupport/common-defs.h: Include time.h before pathmax.h to
avoid compile errors.
diff --git a/gdb/maint.c b/gdb/maint.c
index ec9f4ab..a253584 100644
--- a/gdb/maint.c
+++ b/gdb/maint.c
@@ -1039,10 +1039,11 @@
auto millis = ticks % 1000;
std::time_t as_time = system_clock::to_time_t (now);
- struct tm *tm = localtime (&as_time);
+ struct tm tm;
+ localtime_r (&as_time, &tm);
char out[100];
- strftime (out, sizeof (out), "%F %H:%M:%S", tm);
+ strftime (out, sizeof (out), "%F %H:%M:%S", &tm);
printf_unfiltered ("%s.%03d - %s\n", out, (int) millis, msg);
}
diff --git a/gdb/nat/linux-osdata.c b/gdb/nat/linux-osdata.c
index ca6acd3..d82c062 100644
--- a/gdb/nat/linux-osdata.c
+++ b/gdb/nat/linux-osdata.c
@@ -916,7 +916,11 @@
{
time_t t = (time_t) seconds;
- strncpy (time, ctime (&t), maxlen);
+ /* Per the ctime_r manpage, this buffer needs to be at least 26
+ characters long. */
+ char buf[30];
+ const char *time_str = ctime_r (&t, buf);
+ strncpy (time, time_str, maxlen);
time[maxlen - 1] = '\0';
}
}
--
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I329bbdc39d5b576f51859ba00f1617e024c30cbd
Gerrit-Change-Number: 475
Gerrit-PatchSet: 6
Gerrit-Owner: Christian Biesinger <cbiesinger@google.com>
Gerrit-Reviewer: Christian Biesinger <cbiesinger@google.com>
Gerrit-Reviewer: Kevin Buettner <kevinb@redhat.com>
Gerrit-MessageType: merged
^ permalink raw reply [flat|nested] 20+ messages in thread
* [review v6] Use ctime_r and localtime_r for threadsafety
2019-10-31 21:06 [review] Use ctime_r and localtime_r if available Christian Biesinger (Code Review)
` (12 preceding siblings ...)
2019-11-15 19:52 ` [pushed] " Sourceware to Gerrit sync (Code Review)
@ 2019-11-16 22:07 ` Christian Biesinger (Code Review)
13 siblings, 0 replies; 20+ messages in thread
From: Christian Biesinger (Code Review) @ 2019-11-16 22:07 UTC (permalink / raw)
To: Christian Biesinger, gdb-patches; +Cc: Kevin Buettner
Christian Biesinger has posted comments on this change.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/475
......................................................................
Patch Set 6:
(1 comment)
| --- gdb/nat/linux-osdata.c
| +++ gdb/nat/linux-osdata.c
| @@ -908,12 +908,18 @@ time_from_time_t (char *time, int maxlen, TIME_T seconds)
| {
| if (!seconds)
| time[0] = '\0';
| else
| {
| time_t t = (time_t) seconds;
|
| - strncpy (time, ctime (&t), maxlen);
| +#ifdef HAVE_CTIME_R
| + char buf[30];
PS3, Line 916:
Done.
| + const char *time_str = ctime_r (&t, buf);
| +#else
| + const char *time_str = ctime (&t);
| +#endif
| + strncpy (time, time_str, maxlen);
| time[maxlen - 1] = '\0';
| }
| }
|
--
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I329bbdc39d5b576f51859ba00f1617e024c30cbd
Gerrit-Change-Number: 475
Gerrit-PatchSet: 6
Gerrit-Owner: Christian Biesinger <cbiesinger@google.com>
Gerrit-Reviewer: Christian Biesinger <cbiesinger@google.com>
Gerrit-Reviewer: Kevin Buettner <kevinb@redhat.com>
Gerrit-Comment-Date: Sat, 16 Nov 2019 22:07:32 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Kevin Buettner <kevinb@redhat.com>
Gerrit-MessageType: comment
^ permalink raw reply [flat|nested] 20+ messages in thread