[RFC] Create src/gdb/common, move signals.c there.

[RFC] Create src/gdb/common, move signals.c there.

[Doug Evans]

Hi.

Some code is shared between gdb and gdbserver, and over time more
code will be shared.  And I think the lack of explicit sharing
is complicating progress.

So I'd like to create a path for further sharing, and am starting
with a modest step to see if there's interest in pursuing this.

This patch does these things:

1) Create src/gdb/common, and adds -I's to $(src)/common to the Makefiles.
2) Pull the duplicate decls of the functions in signals.c out of
   gdb/target.h and gdbserver/server.h and put them in common/gdb_signals.h.
3) Move signals/signals.c to common/signals.c, and cleans up signals.c
   a bit.

I recognize that manually configuring gdbserver and building it apart
from gdb is explicitly supported, and this patch doesn't change this.
[Nor am I suggesting breaking this,
e.g. I'm not proposing creating libcommon.a.]

One thing I would like to have done in this patch is remove the #include's of
defs.h and server.h from signals.c, but that involves making gdb_string.h
work in both gdb and gdbserver.  I think this is a reasonable goal,
(modulo having gdb and gdbserver go to a gnulib version, if that makes sense)
but it requires a bit of effort (e.g. gdb/configure.ac and
gdbserver/configure.ac script hacking and how much of them to make common,
either as duplication or as a separate .m4 file or some such),
and I didn't want to put too much effort into this until there's general
agreement in principle of the path.

Further steps that can be taken:
[whether any particular suggestion is worth it is debatable,
the point here is to enumerate the steps that _could_ be taken,
and no claim is made that this list is complete]

- move version.h to common,
  allowing us to remove the duplication in server.h
- pull the gdb-specific functions out of signals.c,
  allowing us to remove the #ifndef GDBSERVER in signals.c
- maybe(!) move regformats to common, though it's big enough
  and standalone enough that I don't have an opinion on it
- create a common place to put things like ATTR_FORMAT, ATTR_NORETURN, etc.
  (and various other duplication b/w defs.h/server.h),
  allowing us to remove the duplication in defs.h/server.h
- similarily for the duplication b/w gdb_string.h and server.h

WDYT?


2009-02-05  Doug Evans  <dje@google.com>

	* Makefile.in (GDB_CFLAGS): Add -I$(srcdir)/common.
	(init.c): signals/ -> common/.
	(signals.o): Update.
	* target.h (target_signal_to_string,target_signal_to_string)
	(target_signal_from_name,target_signal_to_host_p)
	(target_signal_from_host,target_signal_to_host): Move to ...
	* common/gdb_signals.h: ... here.  New file.

	* gdbserver/Makefile.in (INCLUDE_CFLAGS): Add -I$(srcdir)/../common.
	(server_h): Add gdb_signals.h.
	(signals.o): Update.
	* server.h (target_signal_from_host,target_signal_to_host_p)
	(target_signal_to_host,target_signal_to_name): Moved to gdb_signals.h.

	* common/signals.c: Moved here from signals/signals.c.
	#include gdb_signals.h, remove #include of target.h in gdb case.
	(target_signal_from_command,default_target_signal_to_host)
	(default_target_signal_from_host): Move inside #ifndef GDBSERVER.

Index: Makefile.in
===================================================================
RCS file: /cvs/src/src/gdb/Makefile.in,v
retrieving revision 1.1065
diff -u -p -u -p -r1.1065 Makefile.in
--- Makefile.in	30 Jan 2009 22:14:52 -0000	1.1065
+++ Makefile.in	5 Feb 2009 20:24:15 -0000
@@ -363,7 +363,8 @@ CONFIG_UNINSTALL = @CONFIG_UNINSTALL@
 # your system doesn't have fcntl.h in /usr/include (which is where it
 # should be according to Posix).
 DEFS = @DEFS@
-GDB_CFLAGS = -I. -I$(srcdir) -I$(srcdir)/config -DLOCALEDIR="\"$(localedir)\"" $(DEFS)
+GDB_CFLAGS = -I. -I$(srcdir) -I$(srcdir)/common -I$(srcdir)/config \
+	-DLOCALEDIR="\"$(localedir)\"" $(DEFS)
 
 # MH_CFLAGS, if defined, has host-dependent CFLAGS from the config directory.
 GLOBAL_CFLAGS = $(MH_CFLAGS)
@@ -1029,7 +1030,7 @@ init.c: $(INIT_FILES)
 	    -e '/^[a-z0-9A-Z_]*_[SU].[co]$$/d' \
 	    -e '/[a-z0-9A-Z_]*-exp.tab.[co]$$/d' \
 	    -e 's/\.[co]$$/.c/' \
-	    -e 's,signals\.c,signals/signals\.c,' \
+	    -e 's,signals\.c,common/signals\.c,' \
 	    -e 's|^\([^  /][^     ]*\)|$(srcdir)/\1|g' | \
 	while read f; do \
 	    sed -n -e 's/^_initialize_\([a-z_0-9A-Z]*\).*/\1/p' $$f 2>/dev/null; \
@@ -1736,13 +1737,13 @@ mi-common.o: $(srcdir)/mi/mi-common.c
 	$(POSTCOMPILE)
 
 #
-# gdb/signals/ dependencies
+# gdb/common/ dependencies
 #
 # Need to explicitly specify the compile rule as make will do nothing
 # or try to compile the object file into the sub-directory.
 
-signals.o: $(srcdir)/signals/signals.c
-	$(COMPILE) $(srcdir)/signals/signals.c
+signals.o: $(srcdir)/common/signals.c
+	$(COMPILE) $(srcdir)/common/signals.c
 	$(POSTCOMPILE)
 
 #
Index: target.h
===================================================================
RCS file: /cvs/src/src/gdb/target.h,v
retrieving revision 1.141
diff -u -p -u -p -r1.141 target.h
--- target.h	1 Feb 2009 23:31:03 -0000	1.141
+++ target.h	5 Feb 2009 20:24:15 -0000
@@ -55,6 +55,7 @@ struct regcache;
 #include "dcache.h"
 #include "memattr.h"
 #include "vec.h"
+#include "gdb_signals.h"
 
 enum strata
   {
@@ -176,15 +177,6 @@ enum inferior_event_type
        'step n' like commands.  */
     INF_EXEC_CONTINUE
   };
-
-/* Return the string for a signal.  */
-extern const char *target_signal_to_string (enum target_signal);
-
-/* Return the name (SIGHUP, etc.) for a signal.  */
-extern const char * target_signal_to_name (enum target_signal);
-
-/* Given a name (SIGHUP, etc.), return its signal.  */
-enum target_signal target_signal_from_name (const char *);
 \f
 /* Target objects which can be transfered using target_read,
    target_write, et cetera.  */
@@ -1307,28 +1299,7 @@ extern int remote_timeout;
 /* This is for native targets which use a unix/POSIX-style waitstatus.  */
 extern void store_waitstatus (struct target_waitstatus *, int);
 
-/* Predicate to target_signal_to_host(). Return non-zero if the enum
-   targ_signal SIGNO has an equivalent ``host'' representation.  */
-/* FIXME: cagney/1999-11-22: The name below was chosen in preference
-   to the shorter target_signal_p() because it is far less ambigious.
-   In this context ``target_signal'' refers to GDB's internal
-   representation of the target's set of signals while ``host signal''
-   refers to the target operating system's signal.  Confused?  */
-
-extern int target_signal_to_host_p (enum target_signal signo);
-
-/* Convert between host signal numbers and enum target_signal's.
-   target_signal_to_host() returns 0 and prints a warning() on GDB's
-   console if SIGNO has no equivalent host representation.  */
-/* FIXME: cagney/1999-11-22: Here ``host'' is used incorrectly, it is
-   refering to the target operating system's signal numbering.
-   Similarly, ``enum target_signal'' is named incorrectly, ``enum
-   gdb_signal'' would probably be better as it is refering to GDB's
-   internal representation of a target operating system's signal.  */
-
-extern enum target_signal target_signal_from_host (int);
-extern int target_signal_to_host (enum target_signal);
-
+/* These are in common/signals.c, but they're only used by gdb.  */
 extern enum target_signal default_target_signal_from_host (struct gdbarch *,
 							   int);
 extern int default_target_signal_to_host (struct gdbarch *, 
@@ -1336,6 +1307,7 @@ extern int default_target_signal_to_host
 
 /* Convert from a number used in a GDB command to an enum target_signal.  */
 extern enum target_signal target_signal_from_command (int);
+/* End of files in common/signals.c.  */
 
 /* Set the show memory breakpoints mode to show, and installs a cleanup
    to restore it back to the current value.  */
Index: gdbserver/Makefile.in
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/Makefile.in,v
retrieving revision 1.68
diff -u -p -u -p -r1.68 Makefile.in
--- gdbserver/Makefile.in	3 Jan 2009 05:57:56 -0000	1.68
+++ gdbserver/Makefile.in	5 Feb 2009 20:24:15 -0000
@@ -74,7 +74,8 @@ INCLUDE_DEP = $$(INCLUDE_DIR)
 # -I. for config files.
 # -I${srcdir} for our headers.
 # -I$(srcdir)/../regformats for regdef.h.
-INCLUDE_CFLAGS = -I. -I${srcdir} -I$(srcdir)/../regformats -I$(INCLUDE_DIR)
+INCLUDE_CFLAGS = -I. -I${srcdir} -I$(srcdir)/../common \
+	-I$(srcdir)/../regformats -I$(INCLUDE_DIR)
 
 # M{H,T}_CFLAGS, if defined, has host- and target-dependent CFLAGS
 # from the config/ directory.
@@ -263,7 +264,7 @@ regdat_sh = $(srcdir)/../regformats/regd
 regdef_h = $(srcdir)/../regformats/regdef.h
 regcache_h = $(srcdir)/regcache.h
 server_h = $(srcdir)/server.h $(regcache_h) config.h $(srcdir)/target.h \
-		$(srcdir)/mem-break.h
+		$(srcdir)/mem-break.h $(srcdir)/../common/gdb_signals.h
 
 hostio.o: hostio.c $(server_h)
 hostio-errno.o: hostio-errno.c $(server_h)
@@ -278,7 +279,7 @@ thread-db.o: thread-db.c $(server_h) $(g
 utils.o: utils.c $(server_h)
 gdbreplay.o: gdbreplay.c config.h
 
-signals.o: ../signals/signals.c $(server_h)
+signals.o: ../common/signals.c $(server_h)
 	$(CC) -c $(CPPFLAGS) $(INTERNAL_CFLAGS) $< -DGDBSERVER
 
 memmem.o: ../gnulib/memmem.c
Index: gdbserver/server.h
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/server.h,v
retrieving revision 1.51
diff -u -p -u -p -r1.51 server.h
--- gdbserver/server.h	19 Jan 2009 00:16:46 -0000	1.51
+++ gdbserver/server.h	5 Feb 2009 20:24:15 -0000
@@ -110,7 +110,7 @@ struct dll_info
 
 #include "regcache.h"
 #include "gdb/signals.h"
-
+#include "gdb_signals.h"
 #include "target.h"
 #include "mem-break.h"
 
@@ -265,12 +265,6 @@ void buffer_xml_printf (struct buffer *b
 #define buffer_grow_str0(BUFFER,STRING)                        \
   buffer_grow (BUFFER, STRING, strlen (STRING) + 1)
 
-/* Functions from ``signals.c''.  */
-enum target_signal target_signal_from_host (int hostsig);
-int target_signal_to_host_p (enum target_signal oursig);
-int target_signal_to_host (enum target_signal oursig);
-const char *target_signal_to_name (enum target_signal);
-
 /* Functions from utils.c */
 
 void *xmalloc (size_t) ATTR_MALLOC;



diff -u -p common/server.c
--- signals/signals.c	2009-01-15 14:07:20.000000000 -0800
+++ common/signals.c	2009-02-05 13:15:43.000000000 -0800
@@ -23,7 +23,6 @@
 #include "server.h"
 #else
 #include "defs.h"
-#include "target.h"
 #include "gdb_string.h"
 #endif
 
@@ -31,6 +30,8 @@
 #include <signal.h>
 #endif
 
+#include "gdb_signals.h"
+
 struct gdbarch;
 
 /* Always use __SIGRTMIN if it's available.  SIGRTMIN is the lowest
@@ -807,6 +808,8 @@ target_signal_to_host (enum target_signa
     return targ_signo;
 }
 
+#ifndef GDBSERVER
+
 /* In some circumstances we allow a command to specify a numeric
    signal.  The idea is to keep these circumstances limited so that
    users (and scripts) develop portable habits.  For comparison,
@@ -824,7 +827,6 @@ target_signal_from_command (int num)
 Use \"info signals\" for a list of symbolic signals.");
 }
 
-#ifndef GDBSERVER
 extern initialize_file_ftype _initialize_signals; /* -Wmissing-prototype */
 
 void
@@ -833,7 +835,6 @@ _initialize_signals (void)
   if (strcmp (signals[TARGET_SIGNAL_LAST].string, "TARGET_SIGNAL_MAGIC") != 0)
     internal_error (__FILE__, __LINE__, "failed internal consistency check");
 }
-#endif
 
 int
 default_target_signal_to_host (struct gdbarch *gdbarch, enum target_signal ts)
@@ -846,3 +847,5 @@ default_target_signal_from_host (struct 
 {
   return target_signal_from_host (signo);
 }
+
+#endif /* ! GDBSERVER */



diff -u -p common/gdb_signals.h
--- /dev/null	2007-10-18 09:27:25.000000000 -0700
+++ common/gdb_signals.h	2009-02-05 11:58:47.000000000 -0800
@@ -0,0 +1,56 @@
+/* Target signal translation functions for GDB.
+   Copyright (C) 1990, 1991, 1992, 1993, 1994, 1995, 1996, 1997, 1998, 1999,
+   2000, 2001, 2002, 2003, 2006, 2007, 2008, 2009
+   Free Software Foundation, Inc.
+   Contributed by Cygnus Support.
+
+   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/>.  */
+
+#ifndef COMMON_GDB_SIGNALS_H
+#define COMMON_GDB_SIGNALS_H
+
+#include "gdb/signals.h"
+
+/* Predicate to target_signal_to_host(). Return non-zero if the enum
+   targ_signal SIGNO has an equivalent ``host'' representation.  */
+/* FIXME: cagney/1999-11-22: The name below was chosen in preference
+   to the shorter target_signal_p() because it is far less ambigious.
+   In this context ``target_signal'' refers to GDB's internal
+   representation of the target's set of signals while ``host signal''
+   refers to the target operating system's signal.  Confused?  */
+extern int target_signal_to_host_p (enum target_signal signo);
+
+/* Convert between host signal numbers and enum target_signal's.
+   target_signal_to_host() returns 0 and prints a warning() on GDB's
+   console if SIGNO has no equivalent host representation.  */
+/* FIXME: cagney/1999-11-22: Here ``host'' is used incorrectly, it is
+   refering to the target operating system's signal numbering.
+   Similarly, ``enum target_signal'' is named incorrectly, ``enum
+   gdb_signal'' would probably be better as it is refering to GDB's
+   internal representation of a target operating system's signal.  */
+extern enum target_signal target_signal_from_host (int);
+extern int target_signal_to_host (enum target_signal);
+
+/* Return the string for a signal.  */
+extern const char *target_signal_to_string (enum target_signal);
+
+/* Return the name (SIGHUP, etc.) for a signal.  */
+extern const char *target_signal_to_name (enum target_signal);
+
+/* Given a name (SIGHUP, etc.), return its signal.  */
+enum target_signal target_signal_from_name (const char *);
+
+#endif /* COMMON_GDB_SIGNALS_H */

Re: [RFC] Create src/gdb/common, move signals.c there.

[Doug Evans]

On Thu, Feb 5, 2009 at 1:52 PM, Doug Evans <dje@google.com> wrote:
> Hi.
>
> Some code is shared between gdb and gdbserver, and over time more
> code will be shared.  And I think the lack of explicit sharing
> is complicating progress.
>
> So I'd like to create a path for further sharing, and am starting
> with a modest step to see if there's interest in pursuing this.
>
> This patch does these things:
>
> 1) Create src/gdb/common, and adds -I's to $(src)/common to the Makefiles.
> 2) Pull the duplicate decls of the functions in signals.c out of
>   gdb/target.h and gdbserver/server.h and put them in common/gdb_signals.h.
> 3) Move signals/signals.c to common/signals.c, and cleans up signals.c
>   a bit.
>
> I recognize that manually configuring gdbserver and building it apart
> from gdb is explicitly supported, and this patch doesn't change this.
> [Nor am I suggesting breaking this,
> e.g. I'm not proposing creating libcommon.a.]
>
> One thing I would like to have done in this patch is remove the #include's of
> defs.h and server.h from signals.c, but that involves making gdb_string.h
> work in both gdb and gdbserver.  I think this is a reasonable goal,
> (modulo having gdb and gdbserver go to a gnulib version, if that makes sense)
> but it requires a bit of effort (e.g. gdb/configure.ac and
> gdbserver/configure.ac script hacking and how much of them to make common,
> either as duplication or as a separate .m4 file or some such),
> and I didn't want to put too much effort into this until there's general
> agreement in principle of the path.
>
> Further steps that can be taken:
> [whether any particular suggestion is worth it is debatable,
> the point here is to enumerate the steps that _could_ be taken,
> and no claim is made that this list is complete]
>
> - move version.h to common,
>  allowing us to remove the duplication in server.h
> - pull the gdb-specific functions out of signals.c,
>  allowing us to remove the #ifndef GDBSERVER in signals.c
> - maybe(!) move regformats to common, though it's big enough
>  and standalone enough that I don't have an opinion on it
> - create a common place to put things like ATTR_FORMAT, ATTR_NORETURN, etc.
>  (and various other duplication b/w defs.h/server.h),
>  allowing us to remove the duplication in defs.h/server.h
> - similarily for the duplication b/w gdb_string.h and server.h
>
> WDYT?
>
>
> 2009-02-05  Doug Evans  <dje@google.com>
>
>        * Makefile.in (GDB_CFLAGS): Add -I$(srcdir)/common.
>        (init.c): signals/ -> common/.
>        (signals.o): Update.
>        * target.h (target_signal_to_string,target_signal_to_string)
>        (target_signal_from_name,target_signal_to_host_p)
>        (target_signal_from_host,target_signal_to_host): Move to ...
>        * common/gdb_signals.h: ... here.  New file.
>
>        * gdbserver/Makefile.in (INCLUDE_CFLAGS): Add -I$(srcdir)/../common.
>        (server_h): Add gdb_signals.h.
>        (signals.o): Update.
>        * server.h (target_signal_from_host,target_signal_to_host_p)
>        (target_signal_to_host,target_signal_to_name): Moved to gdb_signals.h.
>
>        * common/signals.c: Moved here from signals/signals.c.
>        #include gdb_signals.h, remove #include of target.h in gdb case.
>        (target_signal_from_command,default_target_signal_to_host)
>        (default_target_signal_from_host): Move inside #ifndef GDBSERVER.
>
> Index: Makefile.in
> ===================================================================
> RCS file: /cvs/src/src/gdb/Makefile.in,v
> retrieving revision 1.1065
> diff -u -p -u -p -r1.1065 Makefile.in
> --- Makefile.in 30 Jan 2009 22:14:52 -0000      1.1065
> +++ Makefile.in 5 Feb 2009 20:24:15 -0000
> @@ -363,7 +363,8 @@ CONFIG_UNINSTALL = @CONFIG_UNINSTALL@
>  # your system doesn't have fcntl.h in /usr/include (which is where it
>  # should be according to Posix).
>  DEFS = @DEFS@
> -GDB_CFLAGS = -I. -I$(srcdir) -I$(srcdir)/config -DLOCALEDIR="\"$(localedir)\"" $(DEFS)
> +GDB_CFLAGS = -I. -I$(srcdir) -I$(srcdir)/common -I$(srcdir)/config \
> +       -DLOCALEDIR="\"$(localedir)\"" $(DEFS)
>
>  # MH_CFLAGS, if defined, has host-dependent CFLAGS from the config directory.
>  GLOBAL_CFLAGS = $(MH_CFLAGS)
> @@ -1029,7 +1030,7 @@ init.c: $(INIT_FILES)
>            -e '/^[a-z0-9A-Z_]*_[SU].[co]$$/d' \
>            -e '/[a-z0-9A-Z_]*-exp.tab.[co]$$/d' \
>            -e 's/\.[co]$$/.c/' \
> -           -e 's,signals\.c,signals/signals\.c,' \
> +           -e 's,signals\.c,common/signals\.c,' \
>            -e 's|^\([^  /][^     ]*\)|$(srcdir)/\1|g' | \
>        while read f; do \
>            sed -n -e 's/^_initialize_\([a-z_0-9A-Z]*\).*/\1/p' $$f 2>/dev/null; \
> @@ -1736,13 +1737,13 @@ mi-common.o: $(srcdir)/mi/mi-common.c
>        $(POSTCOMPILE)
>
>  #
> -# gdb/signals/ dependencies
> +# gdb/common/ dependencies
>  #
>  # Need to explicitly specify the compile rule as make will do nothing
>  # or try to compile the object file into the sub-directory.
>
> -signals.o: $(srcdir)/signals/signals.c
> -       $(COMPILE) $(srcdir)/signals/signals.c
> +signals.o: $(srcdir)/common/signals.c
> +       $(COMPILE) $(srcdir)/common/signals.c
>        $(POSTCOMPILE)
>
>  #
> Index: target.h
> ===================================================================
> RCS file: /cvs/src/src/gdb/target.h,v
> retrieving revision 1.141
> diff -u -p -u -p -r1.141 target.h
> --- target.h    1 Feb 2009 23:31:03 -0000       1.141
> +++ target.h    5 Feb 2009 20:24:15 -0000
> @@ -55,6 +55,7 @@ struct regcache;
>  #include "dcache.h"
>  #include "memattr.h"
>  #include "vec.h"
> +#include "gdb_signals.h"
>
>  enum strata
>   {
> @@ -176,15 +177,6 @@ enum inferior_event_type
>        'step n' like commands.  */
>     INF_EXEC_CONTINUE
>   };
> -
> -/* Return the string for a signal.  */
> -extern const char *target_signal_to_string (enum target_signal);
> -
> -/* Return the name (SIGHUP, etc.) for a signal.  */
> -extern const char * target_signal_to_name (enum target_signal);
> -
> -/* Given a name (SIGHUP, etc.), return its signal.  */
> -enum target_signal target_signal_from_name (const char *);
>
>  /* Target objects which can be transfered using target_read,
>    target_write, et cetera.  */
> @@ -1307,28 +1299,7 @@ extern int remote_timeout;
>  /* This is for native targets which use a unix/POSIX-style waitstatus.  */
>  extern void store_waitstatus (struct target_waitstatus *, int);
>
> -/* Predicate to target_signal_to_host(). Return non-zero if the enum
> -   targ_signal SIGNO has an equivalent ``host'' representation.  */
> -/* FIXME: cagney/1999-11-22: The name below was chosen in preference
> -   to the shorter target_signal_p() because it is far less ambigious.
> -   In this context ``target_signal'' refers to GDB's internal
> -   representation of the target's set of signals while ``host signal''
> -   refers to the target operating system's signal.  Confused?  */
> -
> -extern int target_signal_to_host_p (enum target_signal signo);
> -
> -/* Convert between host signal numbers and enum target_signal's.
> -   target_signal_to_host() returns 0 and prints a warning() on GDB's
> -   console if SIGNO has no equivalent host representation.  */
> -/* FIXME: cagney/1999-11-22: Here ``host'' is used incorrectly, it is
> -   refering to the target operating system's signal numbering.
> -   Similarly, ``enum target_signal'' is named incorrectly, ``enum
> -   gdb_signal'' would probably be better as it is refering to GDB's
> -   internal representation of a target operating system's signal.  */
> -
> -extern enum target_signal target_signal_from_host (int);
> -extern int target_signal_to_host (enum target_signal);
> -
> +/* These are in common/signals.c, but they're only used by gdb.  */
>  extern enum target_signal default_target_signal_from_host (struct gdbarch *,
>                                                           int);
>  extern int default_target_signal_to_host (struct gdbarch *,
> @@ -1336,6 +1307,7 @@ extern int default_target_signal_to_host
>
>  /* Convert from a number used in a GDB command to an enum target_signal.  */
>  extern enum target_signal target_signal_from_command (int);
> +/* End of files in common/signals.c.  */
>
>  /* Set the show memory breakpoints mode to show, and installs a cleanup
>    to restore it back to the current value.  */
> Index: gdbserver/Makefile.in
> ===================================================================
> RCS file: /cvs/src/src/gdb/gdbserver/Makefile.in,v
> retrieving revision 1.68
> diff -u -p -u -p -r1.68 Makefile.in
> --- gdbserver/Makefile.in       3 Jan 2009 05:57:56 -0000       1.68
> +++ gdbserver/Makefile.in       5 Feb 2009 20:24:15 -0000
> @@ -74,7 +74,8 @@ INCLUDE_DEP = $$(INCLUDE_DIR)
>  # -I. for config files.
>  # -I${srcdir} for our headers.
>  # -I$(srcdir)/../regformats for regdef.h.
> -INCLUDE_CFLAGS = -I. -I${srcdir} -I$(srcdir)/../regformats -I$(INCLUDE_DIR)
> +INCLUDE_CFLAGS = -I. -I${srcdir} -I$(srcdir)/../common \
> +       -I$(srcdir)/../regformats -I$(INCLUDE_DIR)
>
>  # M{H,T}_CFLAGS, if defined, has host- and target-dependent CFLAGS
>  # from the config/ directory.
> @@ -263,7 +264,7 @@ regdat_sh = $(srcdir)/../regformats/regd
>  regdef_h = $(srcdir)/../regformats/regdef.h
>  regcache_h = $(srcdir)/regcache.h
>  server_h = $(srcdir)/server.h $(regcache_h) config.h $(srcdir)/target.h \
> -               $(srcdir)/mem-break.h
> +               $(srcdir)/mem-break.h $(srcdir)/../common/gdb_signals.h
>
>  hostio.o: hostio.c $(server_h)
>  hostio-errno.o: hostio-errno.c $(server_h)
> @@ -278,7 +279,7 @@ thread-db.o: thread-db.c $(server_h) $(g
>  utils.o: utils.c $(server_h)
>  gdbreplay.o: gdbreplay.c config.h
>
> -signals.o: ../signals/signals.c $(server_h)
> +signals.o: ../common/signals.c $(server_h)
>        $(CC) -c $(CPPFLAGS) $(INTERNAL_CFLAGS) $< -DGDBSERVER
>
>  memmem.o: ../gnulib/memmem.c
> Index: gdbserver/server.h
> ===================================================================
> RCS file: /cvs/src/src/gdb/gdbserver/server.h,v
> retrieving revision 1.51
> diff -u -p -u -p -r1.51 server.h
> --- gdbserver/server.h  19 Jan 2009 00:16:46 -0000      1.51
> +++ gdbserver/server.h  5 Feb 2009 20:24:15 -0000
> @@ -110,7 +110,7 @@ struct dll_info
>
>  #include "regcache.h"
>  #include "gdb/signals.h"
> -
> +#include "gdb_signals.h"
>  #include "target.h"
>  #include "mem-break.h"
>
> @@ -265,12 +265,6 @@ void buffer_xml_printf (struct buffer *b
>  #define buffer_grow_str0(BUFFER,STRING)                        \
>   buffer_grow (BUFFER, STRING, strlen (STRING) + 1)
>
> -/* Functions from ``signals.c''.  */
> -enum target_signal target_signal_from_host (int hostsig);
> -int target_signal_to_host_p (enum target_signal oursig);
> -int target_signal_to_host (enum target_signal oursig);
> -const char *target_signal_to_name (enum target_signal);
> -
>  /* Functions from utils.c */
>
>  void *xmalloc (size_t) ATTR_MALLOC;
>
>
>
> diff -u -p common/server.c
> --- signals/signals.c   2009-01-15 14:07:20.000000000 -0800
> +++ common/signals.c    2009-02-05 13:15:43.000000000 -0800
> @@ -23,7 +23,6 @@
>  #include "server.h"
>  #else
>  #include "defs.h"
> -#include "target.h"
>  #include "gdb_string.h"
>  #endif
>
> @@ -31,6 +30,8 @@
>  #include <signal.h>
>  #endif
>
> +#include "gdb_signals.h"
> +
>  struct gdbarch;
>
>  /* Always use __SIGRTMIN if it's available.  SIGRTMIN is the lowest
> @@ -807,6 +808,8 @@ target_signal_to_host (enum target_signa
>     return targ_signo;
>  }
>
> +#ifndef GDBSERVER
> +
>  /* In some circumstances we allow a command to specify a numeric
>    signal.  The idea is to keep these circumstances limited so that
>    users (and scripts) develop portable habits.  For comparison,
> @@ -824,7 +827,6 @@ target_signal_from_command (int num)
>  Use \"info signals\" for a list of symbolic signals.");
>  }
>
> -#ifndef GDBSERVER
>  extern initialize_file_ftype _initialize_signals; /* -Wmissing-prototype */
>
>  void
> @@ -833,7 +835,6 @@ _initialize_signals (void)
>   if (strcmp (signals[TARGET_SIGNAL_LAST].string, "TARGET_SIGNAL_MAGIC") != 0)
>     internal_error (__FILE__, __LINE__, "failed internal consistency check");
>  }
> -#endif
>
>  int
>  default_target_signal_to_host (struct gdbarch *gdbarch, enum target_signal ts)
> @@ -846,3 +847,5 @@ default_target_signal_from_host (struct
>  {
>   return target_signal_from_host (signo);
>  }
> +
> +#endif /* ! GDBSERVER */
>
>
>
> diff -u -p common/gdb_signals.h
> --- /dev/null   2007-10-18 09:27:25.000000000 -0700
> +++ common/gdb_signals.h        2009-02-05 11:58:47.000000000 -0800
> @@ -0,0 +1,56 @@
> +/* Target signal translation functions for GDB.
> +   Copyright (C) 1990, 1991, 1992, 1993, 1994, 1995, 1996, 1997, 1998, 1999,
> +   2000, 2001, 2002, 2003, 2006, 2007, 2008, 2009
> +   Free Software Foundation, Inc.
> +   Contributed by Cygnus Support.
> +
> +   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/>.  */
> +
> +#ifndef COMMON_GDB_SIGNALS_H
> +#define COMMON_GDB_SIGNALS_H
> +
> +#include "gdb/signals.h"
> +
> +/* Predicate to target_signal_to_host(). Return non-zero if the enum
> +   targ_signal SIGNO has an equivalent ``host'' representation.  */
> +/* FIXME: cagney/1999-11-22: The name below was chosen in preference
> +   to the shorter target_signal_p() because it is far less ambigious.
> +   In this context ``target_signal'' refers to GDB's internal
> +   representation of the target's set of signals while ``host signal''
> +   refers to the target operating system's signal.  Confused?  */
> +extern int target_signal_to_host_p (enum target_signal signo);
> +
> +/* Convert between host signal numbers and enum target_signal's.
> +   target_signal_to_host() returns 0 and prints a warning() on GDB's
> +   console if SIGNO has no equivalent host representation.  */
> +/* FIXME: cagney/1999-11-22: Here ``host'' is used incorrectly, it is
> +   refering to the target operating system's signal numbering.
> +   Similarly, ``enum target_signal'' is named incorrectly, ``enum
> +   gdb_signal'' would probably be better as it is refering to GDB's
> +   internal representation of a target operating system's signal.  */
> +extern enum target_signal target_signal_from_host (int);
> +extern int target_signal_to_host (enum target_signal);
> +
> +/* Return the string for a signal.  */
> +extern const char *target_signal_to_string (enum target_signal);
> +
> +/* Return the name (SIGHUP, etc.) for a signal.  */
> +extern const char *target_signal_to_name (enum target_signal);
> +
> +/* Given a name (SIGHUP, etc.), return its signal.  */
> +enum target_signal target_signal_from_name (const char *);
> +
> +#endif /* COMMON_GDB_SIGNALS_H */
>


Ping.

For reference sake,
whether any particular tidbit is made common needs to be decided on
its own merits.
But I think the concept is sound, and this patch is standalone enough
to warrant inclusion as is.

Ok to check in?

Re: [RFC] Create src/gdb/common, move signals.c there.

[teawater]

I think this idea is good. And I agree with let gdb and gdbserver just
share the code. It's friendly to cross compile environment.

Hui


On Wed, Feb 25, 2009 at 04:22, Doug Evans <dje@google.com> wrote:
> On Thu, Feb 5, 2009 at 1:52 PM, Doug Evans <dje@google.com> wrote:
>> Hi.
>>
>> Some code is shared between gdb and gdbserver, and over time more
>> code will be shared.  And I think the lack of explicit sharing
>> is complicating progress.
>>
>> So I'd like to create a path for further sharing, and am starting
>> with a modest step to see if there's interest in pursuing this.
>>
>> This patch does these things:
>>
>> 1) Create src/gdb/common, and adds -I's to $(src)/common to the Makefiles.
>> 2) Pull the duplicate decls of the functions in signals.c out of
>>   gdb/target.h and gdbserver/server.h and put them in common/gdb_signals.h.
>> 3) Move signals/signals.c to common/signals.c, and cleans up signals.c
>>   a bit.
>>
>> I recognize that manually configuring gdbserver and building it apart
>> from gdb is explicitly supported, and this patch doesn't change this.
>> [Nor am I suggesting breaking this,
>> e.g. I'm not proposing creating libcommon.a.]
>>
>> One thing I would like to have done in this patch is remove the #include's of
>> defs.h and server.h from signals.c, but that involves making gdb_string.h
>> work in both gdb and gdbserver.  I think this is a reasonable goal,
>> (modulo having gdb and gdbserver go to a gnulib version, if that makes sense)
>> but it requires a bit of effort (e.g. gdb/configure.ac and
>> gdbserver/configure.ac script hacking and how much of them to make common,
>> either as duplication or as a separate .m4 file or some such),
>> and I didn't want to put too much effort into this until there's general
>> agreement in principle of the path.
>>
>> Further steps that can be taken:
>> [whether any particular suggestion is worth it is debatable,
>> the point here is to enumerate the steps that _could_ be taken,
>> and no claim is made that this list is complete]
>>
>> - move version.h to common,
>>  allowing us to remove the duplication in server.h
>> - pull the gdb-specific functions out of signals.c,
>>  allowing us to remove the #ifndef GDBSERVER in signals.c
>> - maybe(!) move regformats to common, though it's big enough
>>  and standalone enough that I don't have an opinion on it
>> - create a common place to put things like ATTR_FORMAT, ATTR_NORETURN, etc.
>>  (and various other duplication b/w defs.h/server.h),
>>  allowing us to remove the duplication in defs.h/server.h
>> - similarily for the duplication b/w gdb_string.h and server.h
>>
>> WDYT?
>>
>>
>> 2009-02-05  Doug Evans  <dje@google.com>
>>
>>        * Makefile.in (GDB_CFLAGS): Add -I$(srcdir)/common.
>>        (init.c): signals/ -> common/.
>>        (signals.o): Update.
>>        * target.h (target_signal_to_string,target_signal_to_string)
>>        (target_signal_from_name,target_signal_to_host_p)
>>        (target_signal_from_host,target_signal_to_host): Move to ...
>>        * common/gdb_signals.h: ... here.  New file.
>>
>>        * gdbserver/Makefile.in (INCLUDE_CFLAGS): Add -I$(srcdir)/../common.
>>        (server_h): Add gdb_signals.h.
>>        (signals.o): Update.
>>        * server.h (target_signal_from_host,target_signal_to_host_p)
>>        (target_signal_to_host,target_signal_to_name): Moved to gdb_signals.h.
>>
>>        * common/signals.c: Moved here from signals/signals.c.
>>        #include gdb_signals.h, remove #include of target.h in gdb case.
>>        (target_signal_from_command,default_target_signal_to_host)
>>        (default_target_signal_from_host): Move inside #ifndef GDBSERVER.
>>
>> Index: Makefile.in
>> ===================================================================
>> RCS file: /cvs/src/src/gdb/Makefile.in,v
>> retrieving revision 1.1065
>> diff -u -p -u -p -r1.1065 Makefile.in
>> --- Makefile.in 30 Jan 2009 22:14:52 -0000      1.1065
>> +++ Makefile.in 5 Feb 2009 20:24:15 -0000
>> @@ -363,7 +363,8 @@ CONFIG_UNINSTALL = @CONFIG_UNINSTALL@
>>  # your system doesn't have fcntl.h in /usr/include (which is where it
>>  # should be according to Posix).
>>  DEFS = @DEFS@
>> -GDB_CFLAGS = -I. -I$(srcdir) -I$(srcdir)/config -DLOCALEDIR="\"$(localedir)\"" $(DEFS)
>> +GDB_CFLAGS = -I. -I$(srcdir) -I$(srcdir)/common -I$(srcdir)/config \
>> +       -DLOCALEDIR="\"$(localedir)\"" $(DEFS)
>>
>>  # MH_CFLAGS, if defined, has host-dependent CFLAGS from the config directory.
>>  GLOBAL_CFLAGS = $(MH_CFLAGS)
>> @@ -1029,7 +1030,7 @@ init.c: $(INIT_FILES)
>>            -e '/^[a-z0-9A-Z_]*_[SU].[co]$$/d' \
>>            -e '/[a-z0-9A-Z_]*-exp.tab.[co]$$/d' \
>>            -e 's/\.[co]$$/.c/' \
>> -           -e 's,signals\.c,signals/signals\.c,' \
>> +           -e 's,signals\.c,common/signals\.c,' \
>>            -e 's|^\([^  /][^     ]*\)|$(srcdir)/\1|g' | \
>>        while read f; do \
>>            sed -n -e 's/^_initialize_\([a-z_0-9A-Z]*\).*/\1/p' $$f 2>/dev/null; \
>> @@ -1736,13 +1737,13 @@ mi-common.o: $(srcdir)/mi/mi-common.c
>>        $(POSTCOMPILE)
>>
>>  #
>> -# gdb/signals/ dependencies
>> +# gdb/common/ dependencies
>>  #
>>  # Need to explicitly specify the compile rule as make will do nothing
>>  # or try to compile the object file into the sub-directory.
>>
>> -signals.o: $(srcdir)/signals/signals.c
>> -       $(COMPILE) $(srcdir)/signals/signals.c
>> +signals.o: $(srcdir)/common/signals.c
>> +       $(COMPILE) $(srcdir)/common/signals.c
>>        $(POSTCOMPILE)
>>
>>  #
>> Index: target.h
>> ===================================================================
>> RCS file: /cvs/src/src/gdb/target.h,v
>> retrieving revision 1.141
>> diff -u -p -u -p -r1.141 target.h
>> --- target.h    1 Feb 2009 23:31:03 -0000       1.141
>> +++ target.h    5 Feb 2009 20:24:15 -0000
>> @@ -55,6 +55,7 @@ struct regcache;
>>  #include "dcache.h"
>>  #include "memattr.h"
>>  #include "vec.h"
>> +#include "gdb_signals.h"
>>
>>  enum strata
>>   {
>> @@ -176,15 +177,6 @@ enum inferior_event_type
>>        'step n' like commands.  */
>>     INF_EXEC_CONTINUE
>>   };
>> -
>> -/* Return the string for a signal.  */
>> -extern const char *target_signal_to_string (enum target_signal);
>> -
>> -/* Return the name (SIGHUP, etc.) for a signal.  */
>> -extern const char * target_signal_to_name (enum target_signal);
>> -
>> -/* Given a name (SIGHUP, etc.), return its signal.  */
>> -enum target_signal target_signal_from_name (const char *);
>>
>>  /* Target objects which can be transfered using target_read,
>>    target_write, et cetera.  */
>> @@ -1307,28 +1299,7 @@ extern int remote_timeout;
>>  /* This is for native targets which use a unix/POSIX-style waitstatus.  */
>>  extern void store_waitstatus (struct target_waitstatus *, int);
>>
>> -/* Predicate to target_signal_to_host(). Return non-zero if the enum
>> -   targ_signal SIGNO has an equivalent ``host'' representation.  */
>> -/* FIXME: cagney/1999-11-22: The name below was chosen in preference
>> -   to the shorter target_signal_p() because it is far less ambigious.
>> -   In this context ``target_signal'' refers to GDB's internal
>> -   representation of the target's set of signals while ``host signal''
>> -   refers to the target operating system's signal.  Confused?  */
>> -
>> -extern int target_signal_to_host_p (enum target_signal signo);
>> -
>> -/* Convert between host signal numbers and enum target_signal's.
>> -   target_signal_to_host() returns 0 and prints a warning() on GDB's
>> -   console if SIGNO has no equivalent host representation.  */
>> -/* FIXME: cagney/1999-11-22: Here ``host'' is used incorrectly, it is
>> -   refering to the target operating system's signal numbering.
>> -   Similarly, ``enum target_signal'' is named incorrectly, ``enum
>> -   gdb_signal'' would probably be better as it is refering to GDB's
>> -   internal representation of a target operating system's signal.  */
>> -
>> -extern enum target_signal target_signal_from_host (int);
>> -extern int target_signal_to_host (enum target_signal);
>> -
>> +/* These are in common/signals.c, but they're only used by gdb.  */
>>  extern enum target_signal default_target_signal_from_host (struct gdbarch *,
>>                                                           int);
>>  extern int default_target_signal_to_host (struct gdbarch *,
>> @@ -1336,6 +1307,7 @@ extern int default_target_signal_to_host
>>
>>  /* Convert from a number used in a GDB command to an enum target_signal.  */
>>  extern enum target_signal target_signal_from_command (int);
>> +/* End of files in common/signals.c.  */
>>
>>  /* Set the show memory breakpoints mode to show, and installs a cleanup
>>    to restore it back to the current value.  */
>> Index: gdbserver/Makefile.in
>> ===================================================================
>> RCS file: /cvs/src/src/gdb/gdbserver/Makefile.in,v
>> retrieving revision 1.68
>> diff -u -p -u -p -r1.68 Makefile.in
>> --- gdbserver/Makefile.in       3 Jan 2009 05:57:56 -0000       1.68
>> +++ gdbserver/Makefile.in       5 Feb 2009 20:24:15 -0000
>> @@ -74,7 +74,8 @@ INCLUDE_DEP = $$(INCLUDE_DIR)
>>  # -I. for config files.
>>  # -I${srcdir} for our headers.
>>  # -I$(srcdir)/../regformats for regdef.h.
>> -INCLUDE_CFLAGS = -I. -I${srcdir} -I$(srcdir)/../regformats -I$(INCLUDE_DIR)
>> +INCLUDE_CFLAGS = -I. -I${srcdir} -I$(srcdir)/../common \
>> +       -I$(srcdir)/../regformats -I$(INCLUDE_DIR)
>>
>>  # M{H,T}_CFLAGS, if defined, has host- and target-dependent CFLAGS
>>  # from the config/ directory.
>> @@ -263,7 +264,7 @@ regdat_sh = $(srcdir)/../regformats/regd
>>  regdef_h = $(srcdir)/../regformats/regdef.h
>>  regcache_h = $(srcdir)/regcache.h
>>  server_h = $(srcdir)/server.h $(regcache_h) config.h $(srcdir)/target.h \
>> -               $(srcdir)/mem-break.h
>> +               $(srcdir)/mem-break.h $(srcdir)/../common/gdb_signals.h
>>
>>  hostio.o: hostio.c $(server_h)
>>  hostio-errno.o: hostio-errno.c $(server_h)
>> @@ -278,7 +279,7 @@ thread-db.o: thread-db.c $(server_h) $(g
>>  utils.o: utils.c $(server_h)
>>  gdbreplay.o: gdbreplay.c config.h
>>
>> -signals.o: ../signals/signals.c $(server_h)
>> +signals.o: ../common/signals.c $(server_h)
>>        $(CC) -c $(CPPFLAGS) $(INTERNAL_CFLAGS) $< -DGDBSERVER
>>
>>  memmem.o: ../gnulib/memmem.c
>> Index: gdbserver/server.h
>> ===================================================================
>> RCS file: /cvs/src/src/gdb/gdbserver/server.h,v
>> retrieving revision 1.51
>> diff -u -p -u -p -r1.51 server.h
>> --- gdbserver/server.h  19 Jan 2009 00:16:46 -0000      1.51
>> +++ gdbserver/server.h  5 Feb 2009 20:24:15 -0000
>> @@ -110,7 +110,7 @@ struct dll_info
>>
>>  #include "regcache.h"
>>  #include "gdb/signals.h"
>> -
>> +#include "gdb_signals.h"
>>  #include "target.h"
>>  #include "mem-break.h"
>>
>> @@ -265,12 +265,6 @@ void buffer_xml_printf (struct buffer *b
>>  #define buffer_grow_str0(BUFFER,STRING)                        \
>>   buffer_grow (BUFFER, STRING, strlen (STRING) + 1)
>>
>> -/* Functions from ``signals.c''.  */
>> -enum target_signal target_signal_from_host (int hostsig);
>> -int target_signal_to_host_p (enum target_signal oursig);
>> -int target_signal_to_host (enum target_signal oursig);
>> -const char *target_signal_to_name (enum target_signal);
>> -
>>  /* Functions from utils.c */
>>
>>  void *xmalloc (size_t) ATTR_MALLOC;
>>
>>
>>
>> diff -u -p common/server.c
>> --- signals/signals.c   2009-01-15 14:07:20.000000000 -0800
>> +++ common/signals.c    2009-02-05 13:15:43.000000000 -0800
>> @@ -23,7 +23,6 @@
>>  #include "server.h"
>>  #else
>>  #include "defs.h"
>> -#include "target.h"
>>  #include "gdb_string.h"
>>  #endif
>>
>> @@ -31,6 +30,8 @@
>>  #include <signal.h>
>>  #endif
>>
>> +#include "gdb_signals.h"
>> +
>>  struct gdbarch;
>>
>>  /* Always use __SIGRTMIN if it's available.  SIGRTMIN is the lowest
>> @@ -807,6 +808,8 @@ target_signal_to_host (enum target_signa
>>     return targ_signo;
>>  }
>>
>> +#ifndef GDBSERVER
>> +
>>  /* In some circumstances we allow a command to specify a numeric
>>    signal.  The idea is to keep these circumstances limited so that
>>    users (and scripts) develop portable habits.  For comparison,
>> @@ -824,7 +827,6 @@ target_signal_from_command (int num)
>>  Use \"info signals\" for a list of symbolic signals.");
>>  }
>>
>> -#ifndef GDBSERVER
>>  extern initialize_file_ftype _initialize_signals; /* -Wmissing-prototype */
>>
>>  void
>> @@ -833,7 +835,6 @@ _initialize_signals (void)
>>   if (strcmp (signals[TARGET_SIGNAL_LAST].string, "TARGET_SIGNAL_MAGIC") != 0)
>>     internal_error (__FILE__, __LINE__, "failed internal consistency check");
>>  }
>> -#endif
>>
>>  int
>>  default_target_signal_to_host (struct gdbarch *gdbarch, enum target_signal ts)
>> @@ -846,3 +847,5 @@ default_target_signal_from_host (struct
>>  {
>>   return target_signal_from_host (signo);
>>  }
>> +
>> +#endif /* ! GDBSERVER */
>>
>>
>>
>> diff -u -p common/gdb_signals.h
>> --- /dev/null   2007-10-18 09:27:25.000000000 -0700
>> +++ common/gdb_signals.h        2009-02-05 11:58:47.000000000 -0800
>> @@ -0,0 +1,56 @@
>> +/* Target signal translation functions for GDB.
>> +   Copyright (C) 1990, 1991, 1992, 1993, 1994, 1995, 1996, 1997, 1998, 1999,
>> +   2000, 2001, 2002, 2003, 2006, 2007, 2008, 2009
>> +   Free Software Foundation, Inc.
>> +   Contributed by Cygnus Support.
>> +
>> +   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/>.  */
>> +
>> +#ifndef COMMON_GDB_SIGNALS_H
>> +#define COMMON_GDB_SIGNALS_H
>> +
>> +#include "gdb/signals.h"
>> +
>> +/* Predicate to target_signal_to_host(). Return non-zero if the enum
>> +   targ_signal SIGNO has an equivalent ``host'' representation.  */
>> +/* FIXME: cagney/1999-11-22: The name below was chosen in preference
>> +   to the shorter target_signal_p() because it is far less ambigious.
>> +   In this context ``target_signal'' refers to GDB's internal
>> +   representation of the target's set of signals while ``host signal''
>> +   refers to the target operating system's signal.  Confused?  */
>> +extern int target_signal_to_host_p (enum target_signal signo);
>> +
>> +/* Convert between host signal numbers and enum target_signal's.
>> +   target_signal_to_host() returns 0 and prints a warning() on GDB's
>> +   console if SIGNO has no equivalent host representation.  */
>> +/* FIXME: cagney/1999-11-22: Here ``host'' is used incorrectly, it is
>> +   refering to the target operating system's signal numbering.
>> +   Similarly, ``enum target_signal'' is named incorrectly, ``enum
>> +   gdb_signal'' would probably be better as it is refering to GDB's
>> +   internal representation of a target operating system's signal.  */
>> +extern enum target_signal target_signal_from_host (int);
>> +extern int target_signal_to_host (enum target_signal);
>> +
>> +/* Return the string for a signal.  */
>> +extern const char *target_signal_to_string (enum target_signal);
>> +
>> +/* Return the name (SIGHUP, etc.) for a signal.  */
>> +extern const char *target_signal_to_name (enum target_signal);
>> +
>> +/* Given a name (SIGHUP, etc.), return its signal.  */
>> +enum target_signal target_signal_from_name (const char *);
>> +
>> +#endif /* COMMON_GDB_SIGNALS_H */
>>
>
>
> Ping.
>
> For reference sake,
> whether any particular tidbit is made common needs to be decided on
> its own merits.
> But I think the concept is sound, and this patch is standalone enough
> to warrant inclusion as is.
>
> Ok to check in?
>

Re: [RFC] Create src/gdb/common, move signals.c there.

[Daniel Jacobowitz]

On Thu, Feb 05, 2009 at 01:52:02PM -0800, Doug Evans wrote:
> Some code is shared between gdb and gdbserver, and over time more
> code will be shared.  And I think the lack of explicit sharing
> is complicating progress.

I, personally, do not think that sharing code between gdb and
gdbserver is very useful.  There's no defined common API that shared
code could rely on, and the internals are pretty different.

> This patch does these things:
> 
> 1) Create src/gdb/common, and adds -I's to $(src)/common to the Makefiles.
> 2) Pull the duplicate decls of the functions in signals.c out of
>    gdb/target.h and gdbserver/server.h and put them in common/gdb_signals.h.
> 3) Move signals/signals.c to common/signals.c, and cleans up signals.c
>    a bit.

This patch seems fine on its own merits, though.

> - maybe(!) move regformats to common, though it's big enough
>   and standalone enough that I don't have an opinion on it

Not sure what the point of this would be, that code is just for
gdbserver.


-- 
Daniel Jacobowitz
CodeSourcery