Re: [PATCH 1/2] Move safe_strerror to common/

[Pedro Alves <palves@redhat.com>]

> 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