This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
[review] Import the time_r gnulib module
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
--
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I53fc861b192940d613ca97f2910b4533c730f667
Gerrit-Change-Number: 613
Gerrit-PatchSet: 1
Gerrit-Owner: Christian Biesinger <cbiesinger@google.com>
Gerrit-Reviewer: Pedro Alves <palves@redhat.com>
Gerrit-Comment-Date: Tue, 12 Nov 2019 13:16:34 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment