diff mbox

[07/15] Remove GDBSERVER use from nat/i386-dregs.c

Message ID 1404902255-11101-8-git-send-email-gbenson@redhat.com
State Changes Requested, archived
Headers

Commit Message

Gary Benson July 9, 2014, 10:37 a.m. UTC 
  This removes the use of GDBSERVER from nat/i386-dregs.c.  Neither
defs.h or server.h are included: the specific files required are
listed instead.  Also, a declaration previously made only outside
of gdbserver is made unconditional.

gdb/
2014-07-09  Gary Benson  <gbenson@redhat.com>

	* gdb/nat/i386-dregs.c: Don't include server.h or defs.h; update
	includes.
	(debug_hw_points): Declare regardless of GDBSERVER.
---
 gdb/ChangeLog        |    6 ++++++
 gdb/nat/i386-dregs.c |   16 ++++++++--------
 2 files changed, 14 insertions(+), 8 deletions(-)

Comments

Tom Tromey July 10, 2014, 5:49 p.m. UTC | #1 
>>>>> "Gary" == Gary Benson <gbenson@redhat.com> writes:

Gary> This removes the use of GDBSERVER from nat/i386-dregs.c.  Neither
Gary> defs.h or server.h are included: the specific files required are
Gary> listed instead.  Also, a declaration previously made only outside
Gary> of gdbserver is made unconditional.

Gary> -#ifdef GDBSERVER
Gary> -#include "server.h"
Gary> -#else
Gary> -#include "defs.h"
Gary> -#include "inferior.h"
Gary> -#endif
Gary> +#include "common-utils.h"

I think this should include config.h first.

Tom
Gary Benson July 11, 2014, 12:53 p.m. UTC | #2 
Tom Tromey wrote:
> >>>>> "Gary" == Gary Benson <gbenson@redhat.com> writes:
> 
> Gary> This removes the use of GDBSERVER from nat/i386-dregs.c.
> Gary> Neither defs.h or server.h are included: the specific files
> Gary> required are listed instead.  Also, a declaration previously
> Gary> made only outside of gdbserver is made unconditional.
> 
> Gary> -#ifdef GDBSERVER
> Gary> -#include "server.h"
> Gary> -#else
> Gary> -#include "defs.h"
> Gary> -#include "inferior.h"
> Gary> -#endif
> Gary> +#include "common-utils.h"
> 
> I think this should include config.h first.

common-utils.h includes it.  Or did you mean the gnulib one?

Thanks,
Gary
Tom Tromey July 11, 2014, 3:35 p.m. UTC | #3 
>>>>> "Gary" == Gary Benson <gbenson@redhat.com> writes:

Gary> -#ifdef GDBSERVER
Gary> -#include "server.h"
Gary> -#else
Gary> -#include "defs.h"
Gary> -#include "inferior.h"
Gary> -#endif
Gary> +#include "common-utils.h"

>> I think this should include config.h first.

Gary> common-utils.h includes it.  Or did you mean the gnulib one?

Nope, not the gnulib one.  I didn't realize that common-utils.h includes
config.h.  That seems weird to me.

I think it's generally better for "random" headers not to include
config.h and to either have a "standard" base header (like defs.h or
server.h) that includes config.h, or to just have explicit includes of
config.h as the first thing in each .c.

I realize it's not a problem arising from your series.
But would you mind just having the .c include config.h explicitly?
Then at some point I'll go through and fix up common-utils.h and its
users.

Tom
Gary Benson July 11, 2014, 4:12 p.m. UTC | #4 
Tom Tromey wrote:
> >>>>> "Gary" == Gary Benson <gbenson@redhat.com> writes:
> 
> Gary> -#ifdef GDBSERVER
> Gary> -#include "server.h"
> Gary> -#else
> Gary> -#include "defs.h"
> Gary> -#include "inferior.h"
> Gary> -#endif
> Gary> +#include "common-utils.h"
> 
> >> I think this should include config.h first.
> 
> Gary> common-utils.h includes it.  Or did you mean the gnulib one?
> 
> Nope, not the gnulib one.  I didn't realize that common-utils.h
> includes config.h.  That seems weird to me.
> 
> I think it's generally better for "random" headers not to include
> config.h and to either have a "standard" base header (like defs.h or
> server.h) that includes config.h, or to just have explicit includes
> of config.h as the first thing in each .c.
> 
> I realize it's not a problem arising from your series.  But would
> you mind just having the .c include config.h explicitly?  Then at
> some point I'll go through and fix up common-utils.h and its users.

Ok, I'll do that.

Cheers,
Gary
Doug Evans July 14, 2014, 6:48 p.m. UTC | #5 
Gary Benson writes:
 > This removes the use of GDBSERVER from nat/i386-dregs.c.  Neither
 > defs.h or server.h are included: the specific files required are
 > listed instead.  Also, a declaration previously made only outside
 > of gdbserver is made unconditional.
 > 
 > gdb/
 > 2014-07-09  Gary Benson  <gbenson@redhat.com>
 > 
 > 	* gdb/nat/i386-dregs.c: Don't include server.h or defs.h; update
 > 	includes.
 > 	(debug_hw_points): Declare regardless of GDBSERVER.

LGTM
diff mbox

Patch

diff --git a/gdb/nat/i386-dregs.c b/gdb/nat/i386-dregs.c
index e3272cd..63a05ea 100644
--- a/gdb/nat/i386-dregs.c
+++ b/gdb/nat/i386-dregs.c
@@ -17,12 +17,14 @@ 
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
-#ifdef GDBSERVER
-#include "server.h"
-#else
-#include "defs.h"
-#include "inferior.h"
-#endif
+#include "common-utils.h"
+#include "common-types.h"
+#include "break-common.h"
+#include "common-debug.h"
+#include "print-utils.h"
+#include "errors.h"
+#include "gdb_locale.h"
+#include "gdb_assert.h"
 #include "i386-dregs.h"
 
 /* Support for hardware watchpoints and breakpoints using the i386
@@ -175,10 +177,8 @@ 
 /* Types of operations supported by i386_handle_nonaligned_watchpoint.  */
 typedef enum { WP_INSERT, WP_REMOVE, WP_COUNT } i386_wp_op_t;
 
-#ifndef GDBSERVER
 /* Whether or not to print the mirrored debug registers.  */
 extern int debug_hw_points;
-#endif
 
 /* Print the values of the mirrored debug registers.  */