On Tuesday, January 13 2015, Pedro Alves wrote:
> On 01/09/2015 10:19 PM, Sergio Durigan Junior wrote:
>> This patch moves safe_strerror from the gdb/{posix,mingw}-hdep.c files
>> to the respective common/{posix,mingw}-strerror.c files. This is a
>> preparation for the next patch, which shares a common code (to disable
>> address space randomization when creating a new inferior).
>>
>> The patch has been regtested on Fedora 20 x86_64, and no regressions
>> were found. I am assuming the MingW modifications are also safe, but
>> I have not checked them too deep.
>
> Note, it's "MinGW", capital G. There are instances in the patch of the typo.
Ops, thanks. Fixed.
>> --- /dev/null
>> +++ b/gdb/common/common.host
>> @@ -0,0 +1,17 @@
>> +# Mapping of configurations into GDB host definitions. This is
>> +# invoked from the autoconf generated configure script.
>
> Copyright header missing. I don't know why configure.host
> doesn't have one. Seems like an oversight.
Yeah, it was just a copy and paste of gdb/configure.host. Fixed.
>> +
>> +# This file sets the following shell variables:
>> +# gdb_host_obs host-specific .o files to include when building GDB
>> +# srv_host_obs likewise, but when building gdbserver
>> +
>> +case "${host}" in
>> +
>> +i[34567]86-*-mingw32*) gdb_host_obs="mingw-hdep.o mingw-strerror.o"
>> + srv_host_obs="mingw-strerror.o"
>> + ;;
>> +x86_64-*-mingw*) gdb_host_obs="mingw-hdep.o mingw-strerror.o"
>> + srv_host_obs="mingw-strerror.o"
>> + ;;
>
> You can merge those two cases. Let's leave gdb-specific details in gdb proper, etc.
> That is, make this file set a common-specific variable, like:
>
> case "${host}" in
>
> *-mingw*) common_host_obs=mingw-strerror.o
> ;;
> *)
> common_host_obs=posix-strerror.o
> ;;
> ecas
>
> And then in configure.ac, do:
>
> . $srcdir/common/common.host
> gdb_host_obs=$gdb_host_obs $common_host_obs
Hm, right. Fixed.
Here's the updated patch.
> diff --git a/gdb/common/common.host b/gdb/common/common.host
> new file mode 100644
> index 0000000..1c3374a
> --- /dev/null
> +++ b/gdb/common/common.host
> @@ -0,0 +1,35 @@
> +# Common object files to include for each host.
> +#
> +# Copyright (C) 2015 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/>.
> +
> +# Mapping of configurations into GDB host definitions.
This comment isn't right.
> This is
> +# invoked from the autoconf generated configure script.
> +
> +# This file sets the following shell variables:
> +# gdb_host_obs host-specific .o files to include when building GDB
> +# srv_host_obs likewise, but when building gdbserver
This one is stale. Should describe common_host_obs.
> +
> +case "${host}" in
> +
> +*-mingw*) common_host_obs=mingw-strerror.o
> + ;;
> +*)
> + common_host_obs=posix-strerror.o
> + ;;
> +
> +esac
> diff --git a/gdb/configure.ac b/gdb/configure.ac
> index ec776d7..7812ec6 100644
> --- a/gdb/configure.ac
> +++ b/gdb/configure.ac
> @@ -195,7 +195,7 @@ esac],[want64=false])dnl
>
> # Provide defaults for some variables set by the per-host and per-target
> # configuration.
> -gdb_host_obs=posix-hdep.o
> +gdb_host_obs="posix-hdep.o"
Unnecessary change.
>
> if test "${target}" = "${host}"; then
> gdb_native=yes
> @@ -204,6 +204,9 @@ else
> fi
>
> . $srcdir/configure.host
> +. $srcdir/common/common.host
> +
> +gdb_host_obs="$gdb_host_obs $common_host_obs"
>
I'd rather that it was the latter two lines that were together, like:
. $srcdir/configure.host
# Add in the common host objects.
. $srcdir/common/common.host
gdb_host_obs="$gdb_host_obs $common_host_obs"
> # Accumulate some settings from configure.tgt over all enabled targets
>
> diff --git a/gdb/configure.host b/gdb/configure.host
> index d07be4b..9f8a917 100644
> --- a/gdb/configure.host
> +++ b/gdb/configure.host
> @@ -7,7 +7,6 @@
> # gdb_host_float_format host's float floatformat, or 0
> # gdb_host_double_format host's double floatformat, or 0
> # gdb_host_long_double_format host's long double floatformat, or 0
> -# gdb_host_obs host-specific .o files to include
>
> # Map host cpu into the config cpu subdirectory name.
> # The default is $host_cpu.
> @@ -105,7 +104,6 @@ i[34567]86-*-netbsdelf* | i[34567]86-*-knetbsd*-gnu)
> gdb_host=nbsdelf ;;
> i[34567]86-*-go32*) gdb_host=go32 ;;
> i[34567]86-*-mingw32*) gdb_host=mingw
> - gdb_host_obs=mingw-hdep.o
> ;;
> i[34567]86-*-msdosdjgpp*) gdb_host=go32 ;;
> i[34567]86-*-linux*) gdb_host=linux ;;
> @@ -181,7 +179,6 @@ x86_64-*-netbsd* | x86_64-*-knetbsd*-gnu)
> gdb_host=nbsd64 ;;
> x86_64-*-openbsd*) gdb_host=obsd64 ;;
> x86_64-*-mingw*) gdb_host=mingw64
> - gdb_host_obs=mingw-hdep.o
> ;;
> x86_64-*-cygwin*) gdb_host=cygwin64 ;;
> m32r*-*-linux*) gdb_host=linux ;;
Given mingw-hdep.o still exists and must be included in the
mingw gdb build, this must be breaking the build there.
Just drop this hunk from the patch. Could you make sure the mingw build
still builds? There are mingw packages in Fedora. (configure
with --host=x86_64-w64-mingw32).
> --- a/gdb/gdbserver/configure.ac
> +++ b/gdb/gdbserver/configure.ac
> @@ -240,7 +240,14 @@ got it
> ;;
> esac
>
> +# Initialize as POSIX. This will change if the host is MinGW.
Drop this comment.
> +
> +srv_host_obs=""
We don't actually need this.
> +
> . ${srcdir}/configure.srv
> +. ${srcdir}/../common/common.host
> +
> +srv_host_obs="$srv_host_obs $common_host_obs"
Like in the GDB version:
. ${srcdir}/configure.srv
# Add in the common host objects.
. ${srcdir}/../common/common.host
srv_host_obs=$common_host_obs
But you can also drop srv_host_obs and write common_host_obs directly below.
>
> if test "${srv_mingwce}" = "yes"; then
> LIBS="$LIBS -lws2"
> @@ -385,7 +392,7 @@ if test "$srv_xmlfiles" != ""; then
> done
> fi
>
> -GDBSERVER_DEPFILES="$srv_regobj $srv_tgtobj $srv_hostio_err_objs $srv_thread_depfiles"
> +GDBSERVER_DEPFILES="$srv_regobj $srv_tgtobj $srv_hostio_err_objs $srv_thread_depfiles $srv_host_obs"
> GDBSERVER_LIBS="$srv_libs"
>
> dnl Check whether the target supports __sync_*_compare_and_swap.
Thanks,
Pedro Alves
@@ -1680,7 +1680,7 @@ ALLDEPFILES = \
m68klinux-nat.c m68klinux-tdep.c \
m88k-tdep.c m88kbsd-nat.c \
microblaze-tdep.c microblaze-linux-tdep.c \
- mingw-hdep.c \
+ mingw-hdep.c common/mingw-strerror.c \
mips-linux-nat.c mips-linux-tdep.c \
mips-sde-tdep.c \
mips-tdep.c \
@@ -1690,7 +1690,7 @@ ALLDEPFILES = \
nios2-tdep.c nios2-linux-tdep.c \
nbsd-nat.c nbsd-tdep.c obsd-nat.c obsd-tdep.c \
somread.c solib-som.c \
- posix-hdep.c \
+ posix-hdep.c common/posix-strerror.c \
ppc-sysv-tdep.c ppc-linux-nat.c ppc-linux-tdep.c ppc64-tdep.c \
ppcfbsd-nat.c ppcfbsd-tdep.c \
ppcnbsd-nat.c ppcnbsd-tdep.c \
@@ -2227,6 +2227,14 @@ common-exceptions.o: ${srcdir}/common/common-exceptions.c
$(COMPILE) $(srcdir)/common/common-exceptions.c
$(POSTCOMPILE)
+posix-strerror.o: ${srcdir}/common/posix-strerror.c
+ $(COMPILE) $(srcdir)/common/posix-strerror.c
+ $(POSTCOMPILE)
+
+mingw-strerror.o: ${srcdir}/common/mingw-strerror.c
+ $(COMPILE) $(srcdir)/common/mingw-strerror.c
+ $(POSTCOMPILE)
+
#
# gdb/target/ dependencies
#
@@ -62,4 +62,10 @@ int xsnprintf (char *str, size_t size, const char *format, ...)
char *savestring (const char *ptr, size_t len);
+/* The strerror() function can return NULL for errno values that are
+ out of range. Provide a "safe" version that always returns a
+ printable string. */
+
+extern char *safe_strerror (int);
+
#endif
new file mode 100644
@@ -0,0 +1,35 @@
+# Common object files to include for each host.
+#
+# Copyright (C) 2015 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/>.
+
+# Mapping of configurations into GDB host definitions. This is
+# invoked from the autoconf generated configure script.
+
+# This file sets the following shell variables:
+# gdb_host_obs host-specific .o files to include when building GDB
+# srv_host_obs likewise, but when building gdbserver
+
+case "${host}" in
+
+*-mingw*) common_host_obs=mingw-strerror.o
+ ;;
+*)
+ common_host_obs=posix-strerror.o
+ ;;
+
+esac
new file mode 100644
@@ -0,0 +1,64 @@
+/* Safe version of strerror for MinGW, for GDB, the GNU debugger.
+
+ Copyright (C) 2006-2015 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. */
+
+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;
+}
new file mode 100644
@@ -0,0 +1,38 @@
+/* Safe version of strerror for POSIX systems for GDB, the GNU debugger.
+
+ Copyright (C) 2006-2015 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"
+
+/* Implementation of safe_strerror as defined in common-utils.h. */
+
+char *
+safe_strerror (int errnum)
+{
+ char *msg;
+
+ msg = strerror (errnum);
+ if (msg == NULL)
+ {
+ static char buf[32];
+
+ xsnprintf (buf, sizeof buf, "(undocumented errno %d)", errnum);
+ msg = buf;
+ }
+ return (msg);
+}
@@ -5298,7 +5298,7 @@ fi
# Provide defaults for some variables set by the per-host and per-target
# configuration.
-gdb_host_obs=posix-hdep.o
+gdb_host_obs="posix-hdep.o"
if test "${target}" = "${host}"; then
gdb_native=yes
@@ -5307,6 +5307,9 @@ else
fi
. $srcdir/configure.host
+. $srcdir/common/common.host
+
+gdb_host_obs="$gdb_host_obs $common_host_obs"
# Accumulate some settings from configure.tgt over all enabled targets
@@ -195,7 +195,7 @@ esac],[want64=false])dnl
# Provide defaults for some variables set by the per-host and per-target
# configuration.
-gdb_host_obs=posix-hdep.o
+gdb_host_obs="posix-hdep.o"
if test "${target}" = "${host}"; then
gdb_native=yes
@@ -204,6 +204,9 @@ else
fi
. $srcdir/configure.host
+. $srcdir/common/common.host
+
+gdb_host_obs="$gdb_host_obs $common_host_obs"
# Accumulate some settings from configure.tgt over all enabled targets
@@ -7,7 +7,6 @@
# gdb_host_float_format host's float floatformat, or 0
# gdb_host_double_format host's double floatformat, or 0
# gdb_host_long_double_format host's long double floatformat, or 0
-# gdb_host_obs host-specific .o files to include
# Map host cpu into the config cpu subdirectory name.
# The default is $host_cpu.
@@ -105,7 +104,6 @@ i[34567]86-*-netbsdelf* | i[34567]86-*-knetbsd*-gnu)
gdb_host=nbsdelf ;;
i[34567]86-*-go32*) gdb_host=go32 ;;
i[34567]86-*-mingw32*) gdb_host=mingw
- gdb_host_obs=mingw-hdep.o
;;
i[34567]86-*-msdosdjgpp*) gdb_host=go32 ;;
i[34567]86-*-linux*) gdb_host=linux ;;
@@ -181,7 +179,6 @@ x86_64-*-netbsd* | x86_64-*-knetbsd*-gnu)
gdb_host=nbsd64 ;;
x86_64-*-openbsd*) gdb_host=obsd64 ;;
x86_64-*-mingw*) gdb_host=mingw64
- gdb_host_obs=mingw-hdep.o
;;
x86_64-*-cygwin*) gdb_host=cygwin64 ;;
m32r*-*-linux*) gdb_host=linux ;;
@@ -603,8 +603,8 @@ BEGIN { doc["strerror"] = "\
Do not use strerror(), instead use safe_strerror()"
category["strerror"] = ari_regression
fix("strerror", "gdb/gdb_string.h", 1)
- fix("strerror", "gdb/mingw-hdep.c", 1)
- fix("strerror", "gdb/posix-hdep.c", 1)
+ fix("strerror", "gdb/common/mingw-strerror.c", 1)
+ fix("strerror", "gdb/common/posix-strerror.c", 1)
}
/(^|[^_[:alnum:]])strerror[[:space:]]*\(/ {
fail("strerror")
@@ -518,6 +518,12 @@ rsp-low.o: ../common/rsp-low.c
common-utils.o: ../common/common-utils.c
$(COMPILE) $<
$(POSTCOMPILE)
+posix-strerror.o: ../common/posix-strerror.c
+ $(COMPILE) $<
+ $(POSTCOMPILE)
+mingw-strerror.o: ../common/mingw-strerror.c
+ $(COMPILE) $<
+ $(POSTCOMPILE)
vec.o: ../common/vec.c
$(COMPILE) $<
$(POSTCOMPILE)
@@ -5537,7 +5537,14 @@ $as_echo "$gdb_cv_m68k_is_coldfire" >&6; }
;;
esac
+# Initialize as POSIX. This will change if the host is MinGW.
+
+srv_host_obs=""
+
. ${srcdir}/configure.srv
+. ${srcdir}/../common/common.host
+
+srv_host_obs="$srv_host_obs $common_host_obs"
if test "${srv_mingwce}" = "yes"; then
LIBS="$LIBS -lws2"
@@ -6034,7 +6041,7 @@ $as_echo "#define USE_XML 1" >>confdefs.h
done
fi
-GDBSERVER_DEPFILES="$srv_regobj $srv_tgtobj $srv_hostio_err_objs $srv_thread_depfiles"
+GDBSERVER_DEPFILES="$srv_regobj $srv_tgtobj $srv_hostio_err_objs $srv_thread_depfiles $srv_host_obs"
GDBSERVER_LIBS="$srv_libs"
{ $as_echo "$as_me:${as_lineno-$LINENO}: checking whether the target supports __sync_*_compare_and_swap" >&5
@@ -240,7 +240,14 @@ got it
;;
esac
+# Initialize as POSIX. This will change if the host is MinGW.
+
+srv_host_obs=""
+
. ${srcdir}/configure.srv
+. ${srcdir}/../common/common.host
+
+srv_host_obs="$srv_host_obs $common_host_obs"
if test "${srv_mingwce}" = "yes"; then
LIBS="$LIBS -lws2"
@@ -385,7 +392,7 @@ if test "$srv_xmlfiles" != ""; then
done
fi
-GDBSERVER_DEPFILES="$srv_regobj $srv_tgtobj $srv_hostio_err_objs $srv_thread_depfiles"
+GDBSERVER_DEPFILES="$srv_regobj $srv_tgtobj $srv_hostio_err_objs $srv_thread_depfiles $srv_host_obs"
GDBSERVER_LIBS="$srv_libs"
dnl Check whether the target supports __sync_*_compare_and_swap.
@@ -35,50 +35,6 @@ static HANDLE sigint_event;
function. */
struct async_signal_handler *sigint_handler;
-/* The strerror() function can return NULL for errno values that are
- out of range. Provide a "safe" version that always returns a
- printable string.
-
- 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. */
-
-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;
-}
-
/* Return an absolute file name of the running GDB, if possible, or
ARGV0 if not. The return value is in malloc'ed storage. */
@@ -22,26 +22,6 @@
#include "gdb_select.h"
-/* The strerror() function can return NULL for errno values that are
- out of range. Provide a "safe" version that always returns a
- printable string. */
-
-char *
-safe_strerror (int errnum)
-{
- char *msg;
-
- msg = strerror (errnum);
- if (msg == NULL)
- {
- static char buf[32];
-
- xsnprintf (buf, sizeof buf, "(undocumented errno %d)", errnum);
- msg = buf;
- }
- return (msg);
-}
-
/* Wrapper for select. Nothing special needed on POSIX platforms. */
int
@@ -42,9 +42,6 @@ ULONGEST strtoulst (const char *num, const char **trailer, int base);
int compare_positive_ints (const void *ap, const void *bp);
int compare_strings (const void *ap, const void *bp);
-/* This is defined in *-hdep.c, e.g., posix-hdep.c. */
-extern char *safe_strerror (int);
-
/* A wrapper for bfd_errmsg to produce a more helpful error message
in the case of bfd_error_file_ambiguously recognized.
MATCHING, if non-NULL, is the corresponding argument to