[review] Use gnulib's strerror_r on MinGW

Message ID gerrit.1573672870000.I9e6d8a752fc398784201f370cafee65e0ea05474@gnutoolchain-gerrit.osci.io
State New, archived
Headers

Commit Message

Simon Marchi (Code Review) Nov. 13, 2019, 7:21 p.m. UTC
  Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/625
......................................................................

Use gnulib's strerror_r on MinGW

There is no need to keep mingw-strerror around; we can just always use
the code from posix-strerror. The main reason we had that code, it
seems, is to handle winsock error codes, but gnulib's version
handles those.

Unfortunately the code can't be moved into common-utils.c because
libinproctrace.so uses common-utils but not gnulib.

gdb/ChangeLog:

2019-11-13  Christian Biesinger  <cbiesinger@google.com>

	* Makefile.in: Replace {posix,mingw}-strerror.c with safe-strerror.c.
	* configure: Regenerate.
	* configure.ac: Don't source common.host.
	* gdbsupport/common.host: Remove.
	* gdbsupport/mingw-strerror.c: Remove.
	* gdbsupport/posix-strerror.c: Rename to...
	* gdbsupport/safe-strerror.c: ...this.

gdb/gdbserver/ChangeLog:

2019-11-13  Christian Biesinger  <cbiesinger@google.com>

	* Makefile.in: Add safe-strerror.c.
	* configure: Regenerate.
	* configure.ac: Don't source common.host.

Change-Id: I9e6d8a752fc398784201f370cafee65e0ea05474
---
M gdb/Makefile.in
M gdb/configure
M gdb/configure.ac
M gdb/gdbserver/Makefile.in
M gdb/gdbserver/configure
M gdb/gdbserver/configure.ac
D gdb/gdbsupport/common.host
D gdb/gdbsupport/mingw-strerror.c
R gdb/gdbsupport/safe-strerror.c
9 files changed, 6 insertions(+), 121 deletions(-)
  

Comments

Pedro Alves Nov. 14, 2019, 5:04 p.m. UTC | #1
Looks good, thanks for doing this.

On 11/13/19 7:21 PM, Christian Biesinger (Code Review) wrote:

> diff --git a/gdb/gdbsupport/posix-strerror.c b/gdb/gdbsupport/safe-strerror.c
> similarity index 100%
> rename from gdb/gdbsupport/posix-strerror.c
> rename to gdb/gdbsupport/safe-strerror.c
> 

I'd just maybe tweak the intro comment in the file.  It currently reads:

 /* Safe version of strerror for POSIX systems for GDB, the GNU debugger.

And it's not longer only used by POSIX systems.

Thanks,
Pedro Alves
  
Simon Marchi (Code Review) Nov. 15, 2019, 6:40 p.m. UTC | #2
Tom Tromey has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/625
......................................................................


Patch Set 1: Code-Review+2

Thank you.  This is ok.

I considered for a bit whether we should just leave in the host-object capability,
even though it would be unused.  But, I think we can resurrect it if we really
need to, and anyway nowadays we seem to be headed away from this approach.
  
Terekhov, Mikhail via Gdb-patches Nov. 15, 2019, 9:27 p.m. UTC | #3
On Thu, Nov 14, 2019 at 9:04 AM Pedro Alves <palves@redhat.com> wrote:
>
> Looks good, thanks for doing this.
>
> On 11/13/19 7:21 PM, Christian Biesinger (Code Review) wrote:
>
> > diff --git a/gdb/gdbsupport/posix-strerror.c b/gdb/gdbsupport/safe-strerror.c
> > similarity index 100%
> > rename from gdb/gdbsupport/posix-strerror.c
> > rename to gdb/gdbsupport/safe-strerror.c
> >
>
> I'd just maybe tweak the intro comment in the file.  It currently reads:
>
>  /* Safe version of strerror for POSIX systems for GDB, the GNU debugger.
>
> And it's not longer only used by POSIX systems.

Thank you, I updated that locally and am pushing this now.

Christian
  

Patch

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 9ca77f6..5a4ffd5 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -986,6 +986,7 @@ 
 	gdbsupport/ptid.c \
 	gdbsupport/rsp-low.c \
 	gdbsupport/run-time-clock.c \
+	gdbsupport/safe-strerror.c \
 	gdbsupport/scoped_mmap.c \
 	gdbsupport/signals.c \
 	gdbsupport/signals-state-save-restore.c \
@@ -2344,9 +2345,7 @@ 
 	xtensa-linux-nat.c \
 	xtensa-linux-tdep.c \
 	xtensa-tdep.c \
-	xtensa-xtregs.c \
-	gdbsupport/mingw-strerror.c \
-	gdbsupport/posix-strerror.c
+	xtensa-xtregs.c
 
 # Some files need explicit build rules (due to -Werror problems) or due
 # to sub-directory fun 'n' games.
diff --git a/gdb/configure b/gdb/configure
index e805903..4afd7f9 100755
--- a/gdb/configure
+++ b/gdb/configure
@@ -6556,10 +6556,6 @@ 
 
 . $srcdir/configure.host
 
-# Add in the common host objects.
-. $srcdir/gdbsupport/common.host
-gdb_host_obs="$gdb_host_obs $common_host_obs"
-
 # Accumulate some settings from configure.tgt over all enabled targets
 
 TARGET_OBS=
diff --git a/gdb/configure.ac b/gdb/configure.ac
index 354bb7b..6ba7c5c 100644
--- a/gdb/configure.ac
+++ b/gdb/configure.ac
@@ -177,10 +177,6 @@ 
 
 . $srcdir/configure.host
 
-# Add in the common host objects.
-. $srcdir/gdbsupport/common.host
-gdb_host_obs="$gdb_host_obs $common_host_obs"
-
 # Accumulate some settings from configure.tgt over all enabled targets
 
 TARGET_OBS=
diff --git a/gdb/gdbserver/Makefile.in b/gdb/gdbserver/Makefile.in
index b9b5b89..16012dd 100644
--- a/gdb/gdbserver/Makefile.in
+++ b/gdb/gdbserver/Makefile.in
@@ -220,6 +220,7 @@ 
 	$(srcdir)/gdbsupport/print-utils.c \
 	$(srcdir)/gdbsupport/ptid.c \
 	$(srcdir)/gdbsupport/rsp-low.c \
+	$(srcdir)/gdbsupport/safe-strerror.c \
 	$(srcdir)/gdbsupport/tdesc.c \
 	$(srcdir)/gdbsupport/xml-utils.c \
 	$(srcdir)/nat/aarch64-sve-linux-ptrace.c \
@@ -266,6 +267,7 @@ 
 	gdbsupport/print-utils.o \
 	gdbsupport/ptid.o \
 	gdbsupport/rsp-low.o \
+	gdbsupport/safe-strerror.o \
 	gdbsupport/signals.o \
 	gdbsupport/signals-state-save-restore.o \
 	gdbsupport/tdesc.o \
diff --git a/gdb/gdbserver/configure b/gdb/gdbserver/configure
index e513fc5..d1d00a4 100755
--- a/gdb/gdbserver/configure
+++ b/gdb/gdbserver/configure
@@ -7825,10 +7825,6 @@ 
 
 . ${srcdir}/configure.srv
 
-# Add in the common host objects.
-. ${srcdir}/../gdbsupport/common.host
-srv_host_obs="$common_host_obs"
-
 if test "${srv_mingwce}" = "yes"; then
   LIBS="$LIBS -lws2"
 elif test "${srv_mingw}" = "yes"; then
@@ -8361,7 +8357,7 @@ 
   done
 fi
 
-GDBSERVER_DEPFILES="$srv_regobj $srv_tgtobj $srv_hostio_err_objs $srv_thread_depfiles $srv_host_obs $srv_selftest_objs"
+GDBSERVER_DEPFILES="$srv_regobj $srv_tgtobj $srv_hostio_err_objs $srv_thread_depfiles $srv_selftest_objs"
 GDBSERVER_LIBS="$srv_libs"
 
 { $as_echo "$as_me:${as_lineno-$LINENO}: checking whether the target supports __sync_*_compare_and_swap" >&5
diff --git a/gdb/gdbserver/configure.ac b/gdb/gdbserver/configure.ac
index 7ebc9c3..07c9bd7 100644
--- a/gdb/gdbserver/configure.ac
+++ b/gdb/gdbserver/configure.ac
@@ -245,10 +245,6 @@ 
 
 . ${srcdir}/configure.srv
 
-# Add in the common host objects.
-. ${srcdir}/../gdbsupport/common.host
-srv_host_obs="$common_host_obs"
-
 if test "${srv_mingwce}" = "yes"; then
   LIBS="$LIBS -lws2"
 elif test "${srv_mingw}" = "yes"; then
@@ -393,7 +389,7 @@ 
   done
 fi
 
-GDBSERVER_DEPFILES="$srv_regobj $srv_tgtobj $srv_hostio_err_objs $srv_thread_depfiles $srv_host_obs $srv_selftest_objs"
+GDBSERVER_DEPFILES="$srv_regobj $srv_tgtobj $srv_hostio_err_objs $srv_thread_depfiles $srv_selftest_objs"
 GDBSERVER_LIBS="$srv_libs"
 
 dnl Check whether the target supports __sync_*_compare_and_swap.
diff --git a/gdb/gdbsupport/common.host b/gdb/gdbsupport/common.host
deleted file mode 100644
index 4839059..0000000
--- a/gdb/gdbsupport/common.host
+++ /dev/null
@@ -1,36 +0,0 @@ 
-# Common object files to include for each host.
-#
-# Copyright (C) 2015-2019 Free Software Foundation, Inc.
-#
-# This file is part of GDB.
-#
-# 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 of the License, 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/>.
-
-# Map host triplet into the common object files to be included by
-# GDB/gdbserver.  This is invoked from the autoconf generated
-# configure script.
-
-# This file sets the following shell variables:
-#  common_host_obs		host-specific .o files to include when building
-#  				GDB/gdbserver
-
-case "${host}" in
-
-*-mingw*)	common_host_obs=gdbsupport/mingw-strerror.o
-		;;
-*)
-		common_host_obs=gdbsupport/posix-strerror.o
-		;;
-
-esac
diff --git a/gdb/gdbsupport/mingw-strerror.c b/gdb/gdbsupport/mingw-strerror.c
deleted file mode 100644
index 893b7ca..0000000
--- a/gdb/gdbsupport/mingw-strerror.c
+++ /dev/null
@@ -1,64 +0,0 @@ 
-/* Safe version of strerror for MinGW, for GDB, the GNU debugger.
-
-   Copyright (C) 2006-2019 Free Software Foundation, Inc.
-
-   This file is part of GDB.
-
-   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 of the License, 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/>.  */
-
-#include "common-defs.h"
-
-#include <windows.h>
-
-/* Implementation of safe_strerror as defined in common-utils.h.
-
-   The Windows runtime implementation of strerror never returns NULL,
-   but does return a useless string for anything above sys_nerr;
-   unfortunately this includes all socket-related error codes.
-   This replacement tries to find a system-provided error message.  */
-
-const char *
-safe_strerror (int errnum)
-{
-  static char *buffer;
-  int len;
-
-  if (errnum >= 0 && errnum < sys_nerr)
-    return strerror (errnum);
-
-  if (buffer)
-    {
-      LocalFree (buffer);
-      buffer = NULL;
-    }
-
-  if (FormatMessage (FORMAT_MESSAGE_ALLOCATE_BUFFER
-		     | FORMAT_MESSAGE_FROM_SYSTEM,
-		     NULL, errnum,
-		     MAKELANGID (LANG_NEUTRAL, SUBLANG_DEFAULT),
-		     (LPTSTR) &buffer, 0, NULL) == 0)
-    {
-      static char buf[32];
-      xsnprintf (buf, sizeof buf, "(undocumented errno %d)", errnum);
-      return buf;
-    }
-
-  /* Windows error messages end with a period and a CR-LF; strip that
-     out.  */
-  len = strlen (buffer);
-  if (len > 3 && strcmp (buffer + len - 3, ".\r\n") == 0)
-    buffer[len - 3] = '\0';
-
-  return buffer;
-}
diff --git a/gdb/gdbsupport/posix-strerror.c b/gdb/gdbsupport/safe-strerror.c
similarity index 100%
rename from gdb/gdbsupport/posix-strerror.c
rename to gdb/gdbsupport/safe-strerror.c