From patchwork Thu Jan 15 07:04:56 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sergio Durigan Junior X-Patchwork-Id: 4692 Received: (qmail 4297 invoked by alias); 15 Jan 2015 07:05:04 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 4276 invoked by uid 89); 15 Jan 2015 07:05:02 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.0 required=5.0 tests=AWL, BAYES_00, SPF_HELO_PASS, SPF_PASS, T_RP_MATCHES_RCVD, URIBL_BLACK autolearn=no version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Thu, 15 Jan 2015 07:04:58 +0000 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id t0F74vNV017620 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL) for ; Thu, 15 Jan 2015 02:04:57 -0500 Received: from localhost (dhcp-10-15-16-169.yyz.redhat.com [10.15.16.169]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t0F74uUa002449 (version=TLSv1/SSLv3 cipher=AES128-GCM-SHA256 bits=128 verify=NO); Thu, 15 Jan 2015 02:04:57 -0500 From: Sergio Durigan Junior To: Pedro Alves Cc: GDB Patches , Gary Benson Subject: Re: [PATCH 1/2] Move safe_strerror to common/ References: <1420841943-24290-1-git-send-email-sergiodj@redhat.com> <1420841943-24290-2-git-send-email-sergiodj@redhat.com> <54B54DDA.7070909@redhat.com> <87oaq2iezj.fsf@redhat.com> <54B6525B.6080308@redhat.com> X-URL: http://blog.sergiodj.net Date: Thu, 15 Jan 2015 02:04:56 -0500 In-Reply-To: <54B6525B.6080308@redhat.com> (Pedro Alves's message of "Wed, 14 Jan 2015 11:26:19 +0000") Message-ID: <874mrsed7r.fsf@redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 X-IsSubscribed: yes Thanks for the review. On Wednesday, January 14 2015, Pedro Alves wrote: >> 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 . >> + >> +# Mapping of configurations into GDB host definitions. > > This comment isn't right. Improved/fixed. >> 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. Fixed. >> + >> +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. Fixed. >> >> 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" > Adjusted. >> # 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). Thanks; I checked and it builds fine. >> --- 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. Done. >> + >> +srv_host_obs="" > > We don't actually need this. Fixed. >> + >> . ${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. I decided to keep srv_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. > > Thanks, > Pedro Alves Here's the updated patch (with an updated ChangeLog). Thanks, diff --git a/gdb/Makefile.in b/gdb/Makefile.in index f970176..57ed361 100644 --- a/gdb/Makefile.in +++ b/gdb/Makefile.in @@ -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 # diff --git a/gdb/common/common-utils.h b/gdb/common/common-utils.h index f110924..60aa030 100644 --- a/gdb/common/common-utils.h +++ b/gdb/common/common-utils.h @@ -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 diff --git a/gdb/common/common.host b/gdb/common/common.host new file mode 100644 index 0000000..4245e7c --- /dev/null +++ b/gdb/common/common.host @@ -0,0 +1,36 @@ +# 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 . + +# Map host CPU 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=mingw-strerror.o + ;; +*) + common_host_obs=posix-strerror.o + ;; + +esac diff --git a/gdb/common/mingw-strerror.c b/gdb/common/mingw-strerror.c new file mode 100644 index 0000000..c62c42d --- /dev/null +++ b/gdb/common/mingw-strerror.c @@ -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 . */ + +#include "common-defs.h" + +#include + +/* 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; +} diff --git a/gdb/common/posix-strerror.c b/gdb/common/posix-strerror.c new file mode 100644 index 0000000..119fcf6 --- /dev/null +++ b/gdb/common/posix-strerror.c @@ -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 . */ + +#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); +} diff --git a/gdb/configure b/gdb/configure index fdcf215..f62d5a0 100755 --- a/gdb/configure +++ b/gdb/configure @@ -5308,6 +5308,10 @@ fi . $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 TARGET_OBS= diff --git a/gdb/configure.ac b/gdb/configure.ac index cc18174..8dd7f8f 100644 --- a/gdb/configure.ac +++ b/gdb/configure.ac @@ -205,6 +205,10 @@ fi . $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 TARGET_OBS= diff --git a/gdb/contrib/ari/gdb_ari.sh b/gdb/contrib/ari/gdb_ari.sh index 8a8cec8..b868a17 100644 --- a/gdb/contrib/ari/gdb_ari.sh +++ b/gdb/contrib/ari/gdb_ari.sh @@ -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") diff --git a/gdb/gdbserver/Makefile.in b/gdb/gdbserver/Makefile.in index 0e442fc..8406ff4 100644 --- a/gdb/gdbserver/Makefile.in +++ b/gdb/gdbserver/Makefile.in @@ -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) diff --git a/gdb/gdbserver/configure b/gdb/gdbserver/configure index 55bd2c5..2240b78 100755 --- a/gdb/gdbserver/configure +++ b/gdb/gdbserver/configure @@ -5539,6 +5539,10 @@ esac . ${srcdir}/configure.srv +# Add in the common host objects. +. ${srcdir}/../common/common.host +srv_host_obs="$common_host_obs" + if test "${srv_mingwce}" = "yes"; then LIBS="$LIBS -lws2" elif test "${srv_mingw}" = "yes"; then @@ -6034,7 +6038,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 diff --git a/gdb/gdbserver/configure.ac b/gdb/gdbserver/configure.ac index 39e3a06..f883adc 100644 --- a/gdb/gdbserver/configure.ac +++ b/gdb/gdbserver/configure.ac @@ -242,6 +242,10 @@ esac . ${srcdir}/configure.srv +# Add in the common host objects. +. ${srcdir}/../common/common.host +srv_host_obs="$common_host_obs" + if test "${srv_mingwce}" = "yes"; then LIBS="$LIBS -lws2" elif test "${srv_mingw}" = "yes"; then @@ -385,7 +389,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. diff --git a/gdb/mingw-hdep.c b/gdb/mingw-hdep.c index c0cc385..a0ed281 100644 --- a/gdb/mingw-hdep.c +++ b/gdb/mingw-hdep.c @@ -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. */ diff --git a/gdb/posix-hdep.c b/gdb/posix-hdep.c index 13cddba..fef5ec1 100644 --- a/gdb/posix-hdep.c +++ b/gdb/posix-hdep.c @@ -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 diff --git a/gdb/utils.h b/gdb/utils.h index 3debde7..e58260c 100644 --- a/gdb/utils.h +++ b/gdb/utils.h @@ -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