From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 1636 invoked by alias); 5 Feb 2009 21:52:19 -0000 Received: (qmail 1628 invoked by uid 22791); 5 Feb 2009 21:52:17 -0000 X-SWARE-Spam-Status: No, hits=-1.0 required=5.0 tests=AWL,BAYES_20,HK_OBFDOM,SPF_PASS X-Spam-Check-By: sourceware.org Received: from smtp-out.google.com (HELO smtp-out.google.com) (216.239.45.13) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 05 Feb 2009 21:52:09 +0000 Received: from zps75.corp.google.com (zps75.corp.google.com [172.25.146.75]) by smtp-out.google.com with ESMTP id n15Lq75L014727 for ; Thu, 5 Feb 2009 13:52:07 -0800 Received: from localhost (ruffy.corp.google.com [172.18.118.116]) by zps75.corp.google.com with ESMTP id n15Lq20s009097; Thu, 5 Feb 2009 13:52:03 -0800 Received: by localhost (Postfix, from userid 67641) id CE0251C7A1E; Thu, 5 Feb 2009 13:52:02 -0800 (PST) To: gdb-patches@sourceware.org Subject: [RFC] Create src/gdb/common, move signals.c there. Message-Id: <20090205215202.CE0251C7A1E@localhost> Date: Thu, 05 Feb 2009 21:52:00 -0000 From: dje@google.com (Doug Evans) X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2009-02/txt/msg00138.txt.bz2 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 * 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 #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 . */ + +#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 */