Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [PATCH 0/4 v6] Common code cleanups (first four)
@ 2014-08-11 15:18 Gary Benson
  2014-08-11 15:18 ` [PATCH 1/4 v6] Introduce common/errors.h Gary Benson
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Gary Benson @ 2014-08-11 15:18 UTC (permalink / raw)
  To: gdb-patches; +Cc: Doug Evans, Pedro Alves

Hi all,

This series contains updated versions of the first four patches
from the common code cleanups series I last posted on August 1.
The rest of the series is awaiting Pedro's input, but these four
patches can go in now if they are approved.

Patch 1 (Introduce common/errors.h) is changed from the previous
version in that:

  - Functions that must be provided by the client code are
    documented as such in their header file comments.

  - common/errors.c is added to SFILES in gdb's Makefile.in.

Patch 2 (Introduce common-types.h) is unchanged from the previous
version.

Patch 3 (Move print-utils.h to common-defs.h) is likewise unchanged.

Patch 4 (Introduce common-debug.h) is changed from the previous
version in that:

  - Functions that must be provided by the client code are
    documented as such in their header file comments.

  - common/common-debug.c is added to SFILES in gdb's Makefile.in.

  - The static function debug_agent_print in agent.c has been renamed
    debug_agent_printf.

  - The various debug_hw_points declarations throughout GDB and
    gdbserver have been replaced with one in common/common-debug.[ch].
    This addresses an issue noted against patch 10 of the previous
    version of this series [1].

Built and regtested on x86_64 RHEL6.5.

Ok to commit?

Thanks,
Gary

--
[1] https://sourceware.org/ml/gdb-patches/2014-08/msg00136.html


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 2/4 v6] Introduce common-types.h
  2014-08-11 15:18 [PATCH 0/4 v6] Common code cleanups (first four) Gary Benson
  2014-08-11 15:18 ` [PATCH 1/4 v6] Introduce common/errors.h Gary Benson
  2014-08-11 15:18 ` [PATCH 3/4 v6] Move print-utils.h to common-defs.h Gary Benson
@ 2014-08-11 15:18 ` Gary Benson
  2014-08-11 16:17 ` [PATCH 4/4 v6] Introduce common-debug.h Gary Benson
  3 siblings, 0 replies; 7+ messages in thread
From: Gary Benson @ 2014-08-11 15:18 UTC (permalink / raw)
  To: gdb-patches; +Cc: Doug Evans, Pedro Alves

This introduces common-types.h.  This file defines various standard
types used by gdb and gdbserver.

Currently these types are conditionally defined based on GDBSERVER.
The long term goal is to remove all such tests; however, this is
difficult as currently gdb uses definitions from BFD.  In the meantime
this is still a step in the right direction.

gdb/
2014-08-11  Tom Tromey  <tromey@redhat.com>
	    Gary Benson  <gbenson@redhat.com>

	* common/common-types.h: New file.
	* Makefile.in (HFILES_NO_SRCDIR): Add common/common-types.h.
	* common/common-defs.h: Include common-types.h.
	* defs.h (gdb_byte, CORE_ADDR, CORE_ADDR_MAX, LONGEST)
	(ULONGEST): Remove.

gdb/gdbserver/
2014-08-11  Tom Tromey  <tromey@redhat.com>
	    Gary Benson  <gbenson@redhat.com>

	* server.h: Add static assertion.
	(gdb_byte, CORE_ADDR, LONGEST, ULONGEST): Remove.
---
 gdb/ChangeLog             |    9 ++++++
 gdb/Makefile.in           |    2 +-
 gdb/common/common-defs.h  |    1 +
 gdb/common/common-types.h |   61 +++++++++++++++++++++++++++++++++++++++++++++
 gdb/defs.h                |   29 ---------------------
 gdb/gdbserver/ChangeLog   |    6 ++++
 gdb/gdbserver/server.h    |   13 +--------
 7 files changed, 80 insertions(+), 41 deletions(-)
 create mode 100644 gdb/common/common-types.h

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index fa0dcd4..76ca0da 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -936,7 +936,7 @@ gdb_bfd.h sparc-ravenscar-thread.h ppc-ravenscar-thread.h nat/linux-btrace.h \
 ctf.h nat/i386-cpuid.h nat/i386-gcc-cpuid.h target/resume.h \
 target/wait.h target/waitstatus.h nat/linux-nat.h nat/linux-waitpid.h \
 common/print-utils.h common/rsp-low.h nat/i386-dregs.h x86-linux-nat.h \
-i386-linux-nat.h common/common-defs.h common/errors.h
+i386-linux-nat.h common/common-defs.h common/errors.h common/common-types.h
 
 # Header files that already have srcdir in them, or which are in objdir.
 
diff --git a/gdb/common/common-defs.h b/gdb/common/common-defs.h
index a15423c..5543e4c 100644
--- a/gdb/common/common-defs.h
+++ b/gdb/common/common-defs.h
@@ -42,5 +42,6 @@
 #include "common-utils.h"
 #include "gdb_assert.h"
 #include "errors.h"
+#include "common-types.h"
 
 #endif /* COMMON_DEFS_H */
diff --git a/gdb/common/common-types.h b/gdb/common/common-types.h
new file mode 100644
index 0000000..9fa1c24
--- /dev/null
+++ b/gdb/common/common-types.h
@@ -0,0 +1,61 @@
+/* Declarations for common types.
+
+   Copyright (C) 1986-2014 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/>.  */
+
+#ifndef COMMON_TYPES_H
+#define COMMON_TYPES_H
+
+#ifdef GDBSERVER
+
+/* * A byte from the program being debugged.  */
+typedef unsigned char gdb_byte;
+
+typedef unsigned long long CORE_ADDR;
+
+typedef long long LONGEST;
+typedef unsigned long long ULONGEST;
+
+#else /* GDBSERVER */
+
+#include "bfd.h"
+
+/* * A byte from the program being debugged.  */
+typedef bfd_byte gdb_byte;
+
+/* * An address in the program being debugged.  Host byte order.  */
+typedef bfd_vma CORE_ADDR;
+
+/* This is to make sure that LONGEST is at least as big as CORE_ADDR.  */
+
+#ifdef BFD64
+
+typedef BFD_HOST_64_BIT LONGEST;
+typedef BFD_HOST_U_64_BIT ULONGEST;
+
+#else /* No BFD64 */
+
+typedef long long LONGEST;
+typedef unsigned long long ULONGEST;
+
+#endif /* No BFD64 */
+#endif /* GDBSERVER */
+
+/* * The largest CORE_ADDR value.  */
+#define CORE_ADDR_MAX (~ (CORE_ADDR) 0)
+
+#endif /* COMMON_TYPES_H */
diff --git a/gdb/defs.h b/gdb/defs.h
index b7271a7..8914512 100644
--- a/gdb/defs.h
+++ b/gdb/defs.h
@@ -76,35 +76,6 @@
 
 #include "hashtab.h"
 
-/* Rather than duplicate all the logic in BFD for figuring out what
-   types to use (which can be pretty complicated), symply define them
-   in terms of the corresponding type from BFD.  */
-
-#include "bfd.h"
-
-/* * A byte from the program being debugged.  */
-typedef bfd_byte gdb_byte;
-
-/* * An address in the program being debugged.  Host byte order.  */
-typedef bfd_vma CORE_ADDR;
-
-/* * The largest CORE_ADDR value.  */
-#define CORE_ADDR_MAX (~ (CORE_ADDR) 0)
-
-/* This is to make sure that LONGEST is at least as big as CORE_ADDR.  */
-
-#ifdef BFD64
-
-#define LONGEST BFD_HOST_64_BIT
-#define ULONGEST BFD_HOST_U_64_BIT
-
-#else /* No BFD64 */
-
-#define LONGEST long long
-#define ULONGEST unsigned long long
-
-#endif /* No BFD64 */
-
 #ifndef min
 #define min(a, b) ((a) < (b) ? (a) : (b))
 #endif
diff --git a/gdb/gdbserver/server.h b/gdb/gdbserver/server.h
index db6ddde..e6b2277 100644
--- a/gdb/gdbserver/server.h
+++ b/gdb/gdbserver/server.h
@@ -21,6 +21,8 @@
 
 #include "common-defs.h"
 
+gdb_static_assert (sizeof (CORE_ADDR) >= sizeof (void *));
+
 #ifdef __MINGW32CE__
 #include "wincecompat.h"
 #endif
@@ -63,19 +65,8 @@ int vsnprintf(char *str, size_t size, const char *format, va_list ap);
 #  define PROG "gdbserver"
 #endif
 
-/* A type used for binary buffers.  */
-typedef unsigned char gdb_byte;
-
 #include "buffer.h"
 #include "xml-utils.h"
-
-/* FIXME: This should probably be autoconf'd for.  It's an integer type at
-   least the size of a (void *).  */
-typedef unsigned long long CORE_ADDR;
-
-typedef long long LONGEST;
-typedef unsigned long long ULONGEST;
-
 #include "regcache.h"
 #include "gdb_signals.h"
 #include "target.h"
-- 
1.7.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 3/4 v6] Move print-utils.h to common-defs.h
  2014-08-11 15:18 [PATCH 0/4 v6] Common code cleanups (first four) Gary Benson
  2014-08-11 15:18 ` [PATCH 1/4 v6] Introduce common/errors.h Gary Benson
@ 2014-08-11 15:18 ` Gary Benson
  2014-08-11 15:18 ` [PATCH 2/4 v6] Introduce common-types.h Gary Benson
  2014-08-11 16:17 ` [PATCH 4/4 v6] Introduce common-debug.h Gary Benson
  3 siblings, 0 replies; 7+ messages in thread
From: Gary Benson @ 2014-08-11 15:18 UTC (permalink / raw)
  To: gdb-patches; +Cc: Doug Evans, Pedro Alves

This commit moves the inclusion of print-utils.h to common-defs.h
and removes all other inclusions.

gdb/
2014-08-11  Gary Benson  <gbenson@redhat.com>

	* common/common-defs.h: Include print-utils.h.
	* utils.h: Do not include print-utils.h.

gdb/gdbserver/
2014-08-11  Gary Benson  <gbenson@redhat.com>

	* utils.h: Do not include print-utils.h.
---
 gdb/ChangeLog            |    5 +++++
 gdb/common/common-defs.h |    1 +
 gdb/gdbserver/ChangeLog  |    4 ++++
 gdb/gdbserver/utils.h    |    2 --
 gdb/utils.h              |    1 -
 5 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/gdb/common/common-defs.h b/gdb/common/common-defs.h
index 5543e4c..66c0d21 100644
--- a/gdb/common/common-defs.h
+++ b/gdb/common/common-defs.h
@@ -43,5 +43,6 @@
 #include "gdb_assert.h"
 #include "errors.h"
 #include "common-types.h"
+#include "print-utils.h"
 
 #endif /* COMMON_DEFS_H */
diff --git a/gdb/gdbserver/utils.h b/gdb/gdbserver/utils.h
index a994f38..cdd80df 100644
--- a/gdb/gdbserver/utils.h
+++ b/gdb/gdbserver/utils.h
@@ -19,8 +19,6 @@
 #ifndef UTILS_H
 #define UTILS_H
 
-#include "print-utils.h"
-
 void fatal (const char *string,...) ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 2);
 char *paddress (CORE_ADDR addr);
 char *pfildes (gdb_fildes_t fd);
diff --git a/gdb/utils.h b/gdb/utils.h
index 18a95a7..57a1c0f 100644
--- a/gdb/utils.h
+++ b/gdb/utils.h
@@ -23,7 +23,6 @@
 
 #include "cleanups.h"
 #include "exceptions.h"
-#include "print-utils.h"
 
 extern void initialize_utils (void);
 
-- 
1.7.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/4 v6] Introduce common/errors.h
  2014-08-11 15:18 [PATCH 0/4 v6] Common code cleanups (first four) Gary Benson
@ 2014-08-11 15:18 ` Gary Benson
  2014-08-11 15:18 ` [PATCH 3/4 v6] Move print-utils.h to common-defs.h Gary Benson
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 7+ messages in thread
From: Gary Benson @ 2014-08-11 15:18 UTC (permalink / raw)
  To: gdb-patches; +Cc: Doug Evans, Pedro Alves

This introduces common/errors.h.  This holds some error- and warning-
related declarations that can be used by the code in common, nat and
target.  Some of the declared functions must be provided by the client
as documented by the header file comments.

gdb/
2014-08-11  Tom Tromey  <tromey@redhat.com>
	    Gary Benson  <gbenson@redhat.com>

	* common/errors.h: New file.
	* common/errors.c: Likewise.
	* Makefile.in (SFILES): Add common/errors.c.
	(HFILES_NO_SRCDIR): Add common/errors.h.
	(COMMON_OBS): Add errors.o.
	(errors.o): New rule.
	* common/common-defs.h: Include errors.h.
	* utils.h (perror_with_name, error, verror, warning, vwarning):
	Don't declare.
	* common/common-utils.h: (malloc_failure, internal_error):
	Likewise.

gdb/gdbserver/
2014-08-11  Tom Tromey  <tromey@redhat.com>
	    Gary Benson  <gbenson@redhat.com>

	* Makefile.in (SFILES): Add common/errors.c.
	(OBS): Add errors.o.
	(IPA_OBS): Add errors-ipa.o.
	(errors.o): New rule.
	(errors-ipa.o): Likewise.
	* utils.h (perror_with_name, error, warning): Don't declare.
	* utils.c (warning): Renamed and rewritten as...
	(vwarning): New function.
	(error): Renamed and rewritten as...
	(verror): New function.
	(internal_error): Renamed and rewritten as...
	(internal_verror): New function.
---
 gdb/ChangeLog             |   15 +++++++++
 gdb/Makefile.in           |   11 +++++--
 gdb/common/common-defs.h  |    1 +
 gdb/common/common-utils.h |    4 --
 gdb/common/errors.c       |   61 ++++++++++++++++++++++++++++++++++++
 gdb/common/errors.h       |   75 +++++++++++++++++++++++++++++++++++++++++++++
 gdb/gdbserver/ChangeLog   |   16 +++++++++
 gdb/gdbserver/Makefile.in |   16 ++++++++--
 gdb/gdbserver/utils.c     |   23 ++++---------
 gdb/gdbserver/utils.h     |    3 --
 gdb/utils.c               |   36 ---------------------
 gdb/utils.h               |   15 ---------
 12 files changed, 196 insertions(+), 80 deletions(-)
 create mode 100644 gdb/common/errors.c
 create mode 100644 gdb/common/errors.h

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 8361030..fa0dcd4 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -850,7 +850,8 @@ SFILES = ada-exp.y ada-lang.c ada-typeprint.c ada-valprint.c ada-tasks.c \
 	common/gdb_vecs.c common/common-utils.c common/xml-utils.c \
 	common/ptid.c common/buffer.c gdb-dlfcn.c common/agent.c \
 	common/format.c common/filestuff.c btrace.c record-btrace.c ctf.c \
-	target/waitstatus.c common/print-utils.c common/rsp-low.c
+	target/waitstatus.c common/print-utils.c common/rsp-low.c \
+	common/errors.c
 
 LINTFILES = $(SFILES) $(YYFILES) $(CONFIG_SRCS) init.c
 
@@ -935,7 +936,7 @@ gdb_bfd.h sparc-ravenscar-thread.h ppc-ravenscar-thread.h nat/linux-btrace.h \
 ctf.h nat/i386-cpuid.h nat/i386-gcc-cpuid.h target/resume.h \
 target/wait.h target/waitstatus.h nat/linux-nat.h nat/linux-waitpid.h \
 common/print-utils.h common/rsp-low.h nat/i386-dregs.h x86-linux-nat.h \
-i386-linux-nat.h common/common-defs.h
+i386-linux-nat.h common/common-defs.h common/errors.h
 
 # Header files that already have srcdir in them, or which are in objdir.
 
@@ -1034,7 +1035,7 @@ COMMON_OBS = $(DEPFILES) $(CONFIG_OBS) $(YYOBJ) \
 	gdb_vecs.o jit.o progspace.o skip.o probe.o \
 	common-utils.o buffer.o ptid.o gdb-dlfcn.o common-agent.o \
 	format.o registry.o btrace.o record-btrace.o waitstatus.o \
-	print-utils.o rsp-low.o
+	print-utils.o rsp-low.o errors.o
 
 TSOBS = inflow.o
 
@@ -2144,6 +2145,10 @@ rsp-low.o: ${srcdir}/common/rsp-low.c
 	$(COMPILE) $(srcdir)/common/rsp-low.c
 	$(POSTCOMPILE)
 
+errors.o: ${srcdir}/common/errors.c
+	$(COMPILE) $(srcdir)/common/errors.c
+	$(POSTCOMPILE)
+
 #
 # gdb/target/ dependencies
 #
diff --git a/gdb/common/common-defs.h b/gdb/common/common-defs.h
index 82290dc..a15423c 100644
--- a/gdb/common/common-defs.h
+++ b/gdb/common/common-defs.h
@@ -41,5 +41,6 @@
 #include "ptid.h"
 #include "common-utils.h"
 #include "gdb_assert.h"
+#include "errors.h"
 
 #endif /* COMMON_DEFS_H */
diff --git a/gdb/common/common-utils.h b/gdb/common/common-utils.h
index 9038bc2..67615ba 100644
--- a/gdb/common/common-utils.h
+++ b/gdb/common/common-utils.h
@@ -38,10 +38,6 @@
 #endif
 #endif
 
-extern void malloc_failure (long size) ATTRIBUTE_NORETURN;
-extern void internal_error (const char *file, int line, const char *, ...)
-     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (3, 4);
-
 /* xmalloc(), xrealloc() and xcalloc() have already been declared in
    "libiberty.h". */
 
diff --git a/gdb/common/errors.c b/gdb/common/errors.c
new file mode 100644
index 0000000..7d0bb6e
--- /dev/null
+++ b/gdb/common/errors.c
@@ -0,0 +1,61 @@
+/* Error reporting facilities.
+
+   Copyright (C) 1986-2014 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/>.  */
+
+#ifdef GDBSERVER
+#include "server.h"
+#else
+#include "defs.h"
+#endif
+#include "errors.h"
+
+/* See common/errors.h.  */
+
+void
+warning (const char *fmt, ...)
+{
+  va_list ap;
+
+  va_start (ap, fmt);
+  vwarning (fmt, ap);
+  va_end (ap);
+}
+
+/* See common/errors.h.  */
+
+void
+error (const char *fmt, ...)
+{
+  va_list ap;
+
+  va_start (ap, fmt);
+  verror (fmt, ap);
+  va_end (ap);
+}
+
+/* See common/errors.h.  */
+
+void
+internal_error (const char *file, int line, const char *fmt, ...)
+{
+  va_list ap;
+
+  va_start (ap, fmt);
+  internal_verror (file, line, fmt, ap);
+  va_end (ap);
+}
diff --git a/gdb/common/errors.h b/gdb/common/errors.h
new file mode 100644
index 0000000..4e6c2b3
--- /dev/null
+++ b/gdb/common/errors.h
@@ -0,0 +1,75 @@
+/* Declarations for error-reporting facilities.
+
+   Copyright (C) 1986-2014 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/>.  */
+
+#ifndef COMMON_ERRORS_H
+#define COMMON_ERRORS_H
+
+/* A problem was detected, but the requested operation can still
+   proceed.  A warning message is constructed using a printf- or
+   vprintf-style argument list.  The function "vwarning" must be
+   provided by the client.  */
+
+extern void warning (const char *fmt, ...)
+     ATTRIBUTE_PRINTF (1, 2);
+
+extern void vwarning (const char *fmt, va_list args)
+     ATTRIBUTE_PRINTF (1, 0);
+
+/* A non-predictable, non-fatal error was detected.  The requested
+   operation cannot proceed.  An error message is constructed using
+   a printf- or vprintf-style argument list.  These functions do not
+   return.  The function "verror" must be provided by the client.  */
+
+extern void error (const char *fmt, ...)
+     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 2);
+
+extern void verror (const char *fmt, va_list args)
+     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 0);
+
+/* An internal error was detected.  Internal errors indicate
+   programming errors such as assertion failures, as opposed to
+   more general errors beyond the application's control.  These
+   functions do not return.  An error message is constructed using
+   a printf- or vprintf-style argument list.  FILE and LINE
+   indicate the file and line number where the programming error
+   was detected.  The function "internal_verror" must be provided
+   by the client.  */
+
+extern void internal_error (const char *file, int line,
+			    const char *fmt, ...)
+     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (3, 4);
+
+extern void internal_verror (const char *file, int line,
+			     const char *fmt, va_list args)
+     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (3, 0);
+\f
+
+/* Like "error", but the error message is constructed by combining
+   STRING with the system error message for errno.  This function does
+   not return.  This function must be provided by the client.  */
+
+extern void perror_with_name (const char *string) ATTRIBUTE_NORETURN;
+
+/* Call this function to handle memory allocation failures.  This
+   function does not return.  This function must be provided by the
+   client.  */
+
+extern void malloc_failure (long size) ATTRIBUTE_NORETURN;
+
+#endif /* COMMON_ERRORS_H */
diff --git a/gdb/gdbserver/Makefile.in b/gdb/gdbserver/Makefile.in
index f9a2f17..1faa00c 100644
--- a/gdb/gdbserver/Makefile.in
+++ b/gdb/gdbserver/Makefile.in
@@ -169,7 +169,7 @@ SFILES=	$(srcdir)/gdbreplay.c $(srcdir)/inferiors.c $(srcdir)/dll.c \
 	$(srcdir)/common/buffer.c $(srcdir)/nat/linux-btrace.c \
 	$(srcdir)/common/filestuff.c $(srcdir)/target/waitstatus.c \
 	$(srcdir)/nat/mips-linux-watch.c $(srcdir)/common/print-utils.c \
-	$(srcdir)/common/rsp-low.c
+	$(srcdir)/common/rsp-low.c $(srcdir)/common/errors.c
 
 DEPFILES = @GDBSERVER_DEPFILES@
 
@@ -182,7 +182,8 @@ OBS = agent.o ax.o inferiors.o regcache.o remote-utils.o server.o signals.o \
       target.o waitstatus.o utils.o debug.o version.o vec.o gdb_vecs.o \
       mem-break.o hostio.o event-loop.o tracepoint.o xml-utils.o \
       common-utils.o ptid.o buffer.o format.o filestuff.o dll.o notif.o \
-      tdesc.o print-utils.o rsp-low.o $(XML_BUILTIN) $(DEPFILES) $(LIBOBJS)
+      tdesc.o print-utils.o rsp-low.o errors.o $(XML_BUILTIN) $(DEPFILES) \
+      $(LIBOBJS)
 GDBREPLAY_OBS = gdbreplay.o version.o
 GDBSERVER_LIBS = @GDBSERVER_LIBS@
 XM_CLIBS = @LIBS@
@@ -300,7 +301,10 @@ gdbreplay$(EXEEXT): $(GDBREPLAY_OBS) $(LIBGNU) $(LIBIBERTY)
 	${CC-LD} $(INTERNAL_CFLAGS) $(INTERNAL_LDFLAGS) -o gdbreplay$(EXEEXT) $(GDBREPLAY_OBS) \
 	  $(XM_CLIBS) $(LIBGNU) $(LIBIBERTY)
 
-IPA_OBJS=ax-ipa.o tracepoint-ipa.o format-ipa.o utils-ipa.o regcache-ipa.o remote-utils-ipa.o common-utils-ipa.o tdesc-ipa.o print-utils-ipa.o rsp-low-ipa.o ${IPA_DEPFILES}
+IPA_OBJS=ax-ipa.o tracepoint-ipa.o format-ipa.o utils-ipa.o \
+	regcache-ipa.o remote-utils-ipa.o common-utils-ipa.o \
+	tdesc-ipa.o print-utils-ipa.o rsp-low-ipa.o errors-ipa.o \
+	${IPA_DEPFILES}
 
 IPA_LIB=libinproctrace.so
 
@@ -489,6 +493,9 @@ print-utils-ipa.o: ../common/print-utils.c
 rsp-low-ipa.o: ../common/rsp-low.c
 	$(IPAGENT_COMPILE) $<
 	$(POSTCOMPILE)
+errors-ipa.o: ../common/errors.c
+	$(IPAGENT_COMPILE) $<
+	$(POSTCOMPILE)
 
 ax.o: ax.c
 	$(COMPILE) $(WARN_CFLAGS_NO_FORMAT) $<
@@ -530,6 +537,9 @@ filestuff.o: ../common/filestuff.c
 agent.o: ../common/agent.c
 	$(COMPILE) $<
 	$(POSTCOMPILE)
+errors.o: ../common/errors.c
+	$(COMPILE) $<
+	$(POSTCOMPILE)
 waitstatus.o: ../target/waitstatus.c
 	$(COMPILE) $<
 	$(POSTCOMPILE)
diff --git a/gdb/gdbserver/utils.c b/gdb/gdbserver/utils.c
index b011637..3aac3cd 100644
--- a/gdb/gdbserver/utils.c
+++ b/gdb/gdbserver/utils.c
@@ -71,18 +71,15 @@ perror_with_name (const char *string)
   error ("%s.", combined);
 }
 
-/* Print an error message and return to command level.
-   STRING is the error message, used as a fprintf string,
-   and ARG is passed as an argument to it.  */
+/* Print an error message and return to top level.  */
 
 void
-error (const char *string,...)
+verror (const char *string, va_list args)
 {
 #ifndef IN_PROCESS_AGENT
   extern jmp_buf toplevel;
 #endif
-  va_list args;
-  va_start (args, string);
+
   fflush (stdout);
   vfprintf (stderr, string, args);
   fprintf (stderr, "\n");
@@ -110,31 +107,25 @@ fatal (const char *string,...)
   exit (1);
 }
 
-/* VARARGS */
+/* Print a warning message.  */
+
 void
-warning (const char *string,...)
+vwarning (const char *string, va_list args)
 {
-  va_list args;
-  va_start (args, string);
   fprintf (stderr, PREFIX);
   vfprintf (stderr, string, args);
   fprintf (stderr, "\n");
-  va_end (args);
 }
 
 /* Report a problem internal to GDBserver, and exit.  */
 
 void
-internal_error (const char *file, int line, const char *fmt, ...)
+internal_verror (const char *file, int line, const char *fmt, va_list args)
 {
-  va_list args;
-  va_start (args, fmt);
-
   fprintf (stderr,  "\
 %s:%d: A problem internal to " TOOLNAME " has been detected.\n", file, line);
   vfprintf (stderr, fmt, args);
   fprintf (stderr, "\n");
-  va_end (args);
   exit (1);
 }
 
diff --git a/gdb/gdbserver/utils.h b/gdb/gdbserver/utils.h
index 906924b..a994f38 100644
--- a/gdb/gdbserver/utils.h
+++ b/gdb/gdbserver/utils.h
@@ -21,10 +21,7 @@
 
 #include "print-utils.h"
 
-void perror_with_name (const char *string) ATTRIBUTE_NORETURN;
-void error (const char *string,...) ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 2);
 void fatal (const char *string,...) ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 2);
-void warning (const char *string,...) ATTRIBUTE_PRINTF (1, 2);
 char *paddress (CORE_ADDR addr);
 char *pfildes (gdb_fildes_t fd);
 
diff --git a/gdb/utils.c b/gdb/utils.c
index 34b842e..2b64023 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -530,22 +530,6 @@ vwarning (const char *string, va_list args)
     }
 }
 
-/* Print a warning message.
-   The first argument STRING is the warning message, used as a fprintf string,
-   and the remaining args are passed as arguments to it.
-   The primary difference between warnings and errors is that a warning
-   does not force the return to command level.  */
-
-void
-warning (const char *string, ...)
-{
-  va_list args;
-
-  va_start (args, string);
-  vwarning (string, args);
-  va_end (args);
-}
-
 /* Print an error message and return to command level.
    The first argument STRING is the error message, used as a fprintf string,
    and the remaining args are passed as arguments to it.  */
@@ -557,16 +541,6 @@ verror (const char *string, va_list args)
 }
 
 void
-error (const char *string, ...)
-{
-  va_list args;
-
-  va_start (args, string);
-  throw_verror (GENERIC_ERROR, string, args);
-  va_end (args);
-}
-
-void
 error_stream (struct ui_file *stream)
 {
   char *message = ui_file_xstrdup (stream, NULL);
@@ -813,16 +787,6 @@ internal_verror (const char *file, int line, const char *fmt, va_list ap)
   throw_quit (_("Command aborted."));
 }
 
-void
-internal_error (const char *file, int line, const char *string, ...)
-{
-  va_list ap;
-
-  va_start (ap, string);
-  internal_verror (file, line, string, ap);
-  va_end (ap);
-}
-
 static struct internal_problem internal_warning_problem = {
   "internal-warning", 1, internal_problem_ask, 1, internal_problem_ask
 };
diff --git a/gdb/utils.h b/gdb/utils.h
index cc79562..18a95a7 100644
--- a/gdb/utils.h
+++ b/gdb/utils.h
@@ -269,7 +269,6 @@ extern void fprintf_symbol_filtered (struct ui_file *, const char *,
 
 extern void throw_perror_with_name (enum errors errcode, const char *string)
   ATTRIBUTE_NORETURN;
-extern void perror_with_name (const char *) ATTRIBUTE_NORETURN;
 
 extern void perror_warning_with_name (const char *string);
 
@@ -283,18 +282,8 @@ extern void (*deprecated_error_begin_hook) (void);
 
 extern char *warning_pre_print;
 
-extern void verror (const char *fmt, va_list ap)
-     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 0);
-
-extern void error (const char *fmt, ...)
-     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 2);
-
 extern void error_stream (struct ui_file *) ATTRIBUTE_NORETURN;
 
-extern void internal_verror (const char *file, int line, const char *,
-			     va_list ap)
-     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (3, 0);
-
 extern void internal_vwarning (const char *file, int line,
 			       const char *, va_list ap)
      ATTRIBUTE_PRINTF (3, 0);
@@ -302,10 +291,6 @@ extern void internal_vwarning (const char *file, int line,
 extern void internal_warning (const char *file, int line,
 			      const char *, ...) ATTRIBUTE_PRINTF (3, 4);
 
-extern void warning (const char *, ...) ATTRIBUTE_PRINTF (1, 2);
-
-extern void vwarning (const char *, va_list args) ATTRIBUTE_PRINTF (1, 0);
-
 extern void demangler_vwarning (const char *file, int line,
 			       const char *, va_list ap)
      ATTRIBUTE_PRINTF (3, 0);
-- 
1.7.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 4/4 v6] Introduce common-debug.h
  2014-08-11 15:18 [PATCH 0/4 v6] Common code cleanups (first four) Gary Benson
                   ` (2 preceding siblings ...)
  2014-08-11 15:18 ` [PATCH 2/4 v6] Introduce common-types.h Gary Benson
@ 2014-08-11 16:17 ` Gary Benson
  2014-08-12 23:52   ` Doug Evans
  3 siblings, 1 reply; 7+ messages in thread
From: Gary Benson @ 2014-08-11 16:17 UTC (permalink / raw)
  To: gdb-patches; +Cc: Doug Evans, Pedro Alves

This introduces common-debug.h.  This holds the flag debug_hw_points
(which can be set to enable debugging of the hardware breakpoint/
watchpoint support code) and debug_printf and debug_vprintf, two
functions that the common code can use to print debugging messages.
Clients of the common code are expected to implement debug_vprintf;
a debug_vprintf function is written from scratch for GDB, and
gdbserver's existing debug_printf is repurposed as debug_vprintf.

common/agent.c is changed to use debug_vprintf rather than
defining the macro DEBUG_AGENT depending on GDBSERVER.

nat/i386-dregs.c is changed to use the externally-implemented
debug_printf, rather than defining it itself, and a now-unnecessary
declaration of debug_hw_points is removed.

Various other files are changed to remove declarations of
debug_hw_points.

gdb/
2014-08-11  Tom Tromey  <tromey@redhat.com>
	    Gary Benson  <gbenson@redhat.com>

	* common/common-debug.h: New file.
	* common/common-debug.c: Likewise.
	* debug.c: Likewise.
	* Makefile.in (SFILES): Add common/common-debug.c.
	(HFILES_NO_SRCDIR): Add common/common-debug.h.
	(COMMON_OBS): Add common-debug.o and debug.o.
	(common-debug.o): New rule.
	* common/common-defs.h: Include common-debug.h.
	* common/agent.c (debug_agent_printf): New function.
	(DEBUG_AGENT): Redefine.
	* nat/i386-dregs.c (debug_printf): Undefine.
	(debug_hw_points): Remove.
	* aarch64-linux-nat.c (debug_hw_points): Likewise.
	* i386-nat.c (debug_hw_points): Likewise.

gdb/gdbserver/
2014-08-11  Tom Tromey  <tromey@redhat.com>
	    Gary Benson  <gbenson@redhat.com>

	* Makefile.in (SFILES): Add common/common-debug.c.
	(OBS): Add common-debug.o.
	(common-debug.o): New rule.
	* debug.h (debug_printf): Don't declare.
	* debug.c (debug_printf): Renamed and rewritten as...
	(debug_vprintf): New function.
	* server.h (debug_hw_points): Remove.
	* server.c (debug_hw_points): Likewise.
	* linux-aarch64-low.c (debug_hw_points): Likewise.
---
 gdb/ChangeLog                     |   18 ++++++++++++++++
 gdb/Makefile.in                   |   11 +++++++--
 gdb/aarch64-linux-nat.c           |    4 ---
 gdb/common/agent.c                |   24 +++++++++++++--------
 gdb/common/common-debug.c         |   41 +++++++++++++++++++++++++++++++++++++
 gdb/common/common-debug.h         |   41 +++++++++++++++++++++++++++++++++++++
 gdb/common/common-defs.h          |    1 +
 gdb/debug.c                       |   28 +++++++++++++++++++++++++
 gdb/gdbserver/ChangeLog           |   13 +++++++++++
 gdb/gdbserver/Makefile.in         |   10 ++++++--
 gdb/gdbserver/debug.c             |   11 +++------
 gdb/gdbserver/debug.h             |    1 -
 gdb/gdbserver/linux-aarch64-low.c |    3 --
 gdb/gdbserver/server.c            |    3 --
 gdb/gdbserver/server.h            |    1 -
 gdb/i386-nat.c                    |    3 --
 gdb/nat/i386-dregs.c              |    9 --------
 17 files changed, 176 insertions(+), 46 deletions(-)
 create mode 100644 gdb/common/common-debug.c
 create mode 100644 gdb/common/common-debug.h
 create mode 100644 gdb/debug.c

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 76ca0da..b33defe 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -851,7 +851,7 @@ SFILES = ada-exp.y ada-lang.c ada-typeprint.c ada-valprint.c ada-tasks.c \
 	common/ptid.c common/buffer.c gdb-dlfcn.c common/agent.c \
 	common/format.c common/filestuff.c btrace.c record-btrace.c ctf.c \
 	target/waitstatus.c common/print-utils.c common/rsp-low.c \
-	common/errors.c
+	common/errors.c common/common-debug.c
 
 LINTFILES = $(SFILES) $(YYFILES) $(CONFIG_SRCS) init.c
 
@@ -936,7 +936,8 @@ gdb_bfd.h sparc-ravenscar-thread.h ppc-ravenscar-thread.h nat/linux-btrace.h \
 ctf.h nat/i386-cpuid.h nat/i386-gcc-cpuid.h target/resume.h \
 target/wait.h target/waitstatus.h nat/linux-nat.h nat/linux-waitpid.h \
 common/print-utils.h common/rsp-low.h nat/i386-dregs.h x86-linux-nat.h \
-i386-linux-nat.h common/common-defs.h common/errors.h common/common-types.h
+i386-linux-nat.h common/common-defs.h common/errors.h common/common-types.h \
+common/common-debug.h
 
 # Header files that already have srcdir in them, or which are in objdir.
 
@@ -1035,7 +1036,7 @@ COMMON_OBS = $(DEPFILES) $(CONFIG_OBS) $(YYOBJ) \
 	gdb_vecs.o jit.o progspace.o skip.o probe.o \
 	common-utils.o buffer.o ptid.o gdb-dlfcn.o common-agent.o \
 	format.o registry.o btrace.o record-btrace.o waitstatus.o \
-	print-utils.o rsp-low.o errors.o
+	print-utils.o rsp-low.o errors.o common-debug.o debug.o
 
 TSOBS = inflow.o
 
@@ -2149,6 +2150,10 @@ errors.o: ${srcdir}/common/errors.c
 	$(COMPILE) $(srcdir)/common/errors.c
 	$(POSTCOMPILE)
 
+common-debug.o: ${srcdir}/common/common-debug.c
+	$(COMPILE) $(srcdir)/common/common-debug.c
+	$(POSTCOMPILE)
+
 #
 # gdb/target/ dependencies
 #
diff --git a/gdb/aarch64-linux-nat.c b/gdb/aarch64-linux-nat.c
index 4ae789b..c5073af 100644
--- a/gdb/aarch64-linux-nat.c
+++ b/gdb/aarch64-linux-nat.c
@@ -119,10 +119,6 @@ get_thread_id (ptid_t ptid)
 static int aarch64_num_bp_regs;
 static int aarch64_num_wp_regs;
 
-/* Debugging of hardware breakpoint/watchpoint support.  */
-
-static int debug_hw_points;
-
 /* Each bit of a variable of this type is used to indicate whether a
    hardware breakpoint or watchpoint setting has been changed since
    the last update.
diff --git a/gdb/common/agent.c b/gdb/common/agent.c
index 3f868ba..2963917 100644
--- a/gdb/common/agent.c
+++ b/gdb/common/agent.c
@@ -31,15 +31,21 @@
 
 int debug_agent = 0;
 
-#ifdef GDBSERVER
-#define DEBUG_AGENT(fmt, args...)	\
-  if (debug_agent)			\
-    fprintf (stderr, fmt, ##args);
-#else
-#define DEBUG_AGENT(fmt, args...)	\
-  if (debug_agent)			\
-    fprintf_unfiltered (gdb_stdlog, fmt, ##args);
-#endif
+/* A stdarg wrapper for debug_vprintf.  */
+
+static void ATTRIBUTE_PRINTF (1, 2)
+debug_agent_printf (const char *fmt, ...)
+{
+  va_list ap;
+
+  if (!debug_agent)
+    return;
+  va_start (ap, fmt);
+  debug_vprintf (fmt, ap);
+  va_end (ap);
+}
+
+#define DEBUG_AGENT debug_agent_printf
 
 /* Global flag to determine using agent or not.  */
 int use_agent = 0;
diff --git a/gdb/common/common-debug.c b/gdb/common/common-debug.c
new file mode 100644
index 0000000..6242c0d
--- /dev/null
+++ b/gdb/common/common-debug.c
@@ -0,0 +1,41 @@
+/* Debug printing functions.
+
+   Copyright (C) 2014 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/>.  */
+
+#ifdef GDBSERVER
+#include "server.h"
+#else
+#include "defs.h"
+#endif
+#include "common-debug.h"
+
+/* See common/common-debug.h.  */
+
+int debug_hw_points;
+
+/* See common/common-debug.h.  */
+
+void
+debug_printf (const char *fmt, ...)
+{
+  va_list ap;
+
+  va_start (ap, fmt);
+  debug_vprintf (fmt, ap);
+  va_end (ap);
+}
diff --git a/gdb/common/common-debug.h b/gdb/common/common-debug.h
new file mode 100644
index 0000000..d63f800
--- /dev/null
+++ b/gdb/common/common-debug.h
@@ -0,0 +1,41 @@
+/* Declarations for debug printing functions.
+
+   Copyright (C) 2014 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/>.  */
+
+#ifndef COMMON_DEBUG_H
+#define COMMON_DEBUG_H
+
+/* Set to nonzero to enable debugging of hardware breakpoint/
+   watchpoint support code.  */
+
+extern int debug_hw_points;
+
+/* Print a formatted message to the appropriate channel for
+   debugging output for the client.  */
+
+extern void debug_printf (const char *format, ...)
+     ATTRIBUTE_PRINTF (1, 2);
+
+/* Print a formatted message to the appropriate channel for
+   debugging output for the client.  This function must be
+   provided by the client.  */
+
+extern void debug_vprintf (const char *format, va_list ap)
+     ATTRIBUTE_PRINTF (1, 0);
+
+#endif /* COMMON_DEBUG_H */
diff --git a/gdb/common/common-defs.h b/gdb/common/common-defs.h
index 66c0d21..2d3444b 100644
--- a/gdb/common/common-defs.h
+++ b/gdb/common/common-defs.h
@@ -44,5 +44,6 @@
 #include "errors.h"
 #include "common-types.h"
 #include "print-utils.h"
+#include "common-debug.h"
 
 #endif /* COMMON_DEFS_H */
diff --git a/gdb/debug.c b/gdb/debug.c
new file mode 100644
index 0000000..95cf617
--- /dev/null
+++ b/gdb/debug.c
@@ -0,0 +1,28 @@
+/* Debug printing functions.
+
+   Copyright (C) 2014 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/>.  */
+
+#include "defs.h"
+
+/* See common/common-debug.h.  */
+
+void
+debug_vprintf (const char *fmt, va_list ap)
+{
+  vfprintf_unfiltered (gdb_stdlog, fmt, ap);
+}
diff --git a/gdb/gdbserver/Makefile.in b/gdb/gdbserver/Makefile.in
index 1faa00c..18486c6 100644
--- a/gdb/gdbserver/Makefile.in
+++ b/gdb/gdbserver/Makefile.in
@@ -169,7 +169,8 @@ SFILES=	$(srcdir)/gdbreplay.c $(srcdir)/inferiors.c $(srcdir)/dll.c \
 	$(srcdir)/common/buffer.c $(srcdir)/nat/linux-btrace.c \
 	$(srcdir)/common/filestuff.c $(srcdir)/target/waitstatus.c \
 	$(srcdir)/nat/mips-linux-watch.c $(srcdir)/common/print-utils.c \
-	$(srcdir)/common/rsp-low.c $(srcdir)/common/errors.c
+	$(srcdir)/common/rsp-low.c $(srcdir)/common/errors.c \
+	$(srcdir)/common/common-debug.c
 
 DEPFILES = @GDBSERVER_DEPFILES@
 
@@ -182,8 +183,8 @@ OBS = agent.o ax.o inferiors.o regcache.o remote-utils.o server.o signals.o \
       target.o waitstatus.o utils.o debug.o version.o vec.o gdb_vecs.o \
       mem-break.o hostio.o event-loop.o tracepoint.o xml-utils.o \
       common-utils.o ptid.o buffer.o format.o filestuff.o dll.o notif.o \
-      tdesc.o print-utils.o rsp-low.o errors.o $(XML_BUILTIN) $(DEPFILES) \
-      $(LIBOBJS)
+      tdesc.o print-utils.o rsp-low.o errors.o common-debug.o \
+      $(XML_BUILTIN) $(DEPFILES) $(LIBOBJS)
 GDBREPLAY_OBS = gdbreplay.o version.o
 GDBSERVER_LIBS = @GDBSERVER_LIBS@
 XM_CLIBS = @LIBS@
@@ -540,6 +541,9 @@ agent.o: ../common/agent.c
 errors.o: ../common/errors.c
 	$(COMPILE) $<
 	$(POSTCOMPILE)
+common-debug.o: ../common/common-debug.c
+	$(COMPILE) $<
+	$(POSTCOMPILE)
 waitstatus.o: ../target/waitstatus.c
 	$(COMPILE) $<
 	$(POSTCOMPILE)
diff --git a/gdb/gdbserver/debug.c b/gdb/gdbserver/debug.c
index c50af76..339e33c 100644
--- a/gdb/gdbserver/debug.c
+++ b/gdb/gdbserver/debug.c
@@ -33,9 +33,8 @@ int debug_timestamp;
    previous call ended with "\n".  */
 
 void
-debug_printf (const char *msg, ...)
+debug_vprintf (const char *format, va_list ap)
 {
-  va_list args;
 #if !defined (IN_PROCESS_AGENT)
   /* N.B. Not thread safe, and can't be used, as is, with IPA.  */
   static int new_line = 1;
@@ -53,13 +52,11 @@ debug_printf (const char *msg, ...)
     }
 #endif
 
-  va_start (args, msg);
-  vfprintf (stderr, msg, args);
-  va_end (args);
+  vfprintf (stderr, format, ap);
 
 #if !defined (IN_PROCESS_AGENT)
-  if (*msg)
-    new_line = msg[strlen (msg) - 1] == '\n';
+  if (*format)
+    new_line = format[strlen (format) - 1] == '\n';
 #endif
 }
 
diff --git a/gdb/gdbserver/debug.h b/gdb/gdbserver/debug.h
index 0f056ca..42a3f21 100644
--- a/gdb/gdbserver/debug.h
+++ b/gdb/gdbserver/debug.h
@@ -29,7 +29,6 @@
 extern int debug_threads;
 extern int debug_timestamp;
 
-void debug_printf (const char *msg, ...) ATTRIBUTE_PRINTF (1, 2);
 void debug_flush (void);
 void do_debug_enter (const char *function_name);
 void do_debug_exit (const char *function_name);
diff --git a/gdb/gdbserver/linux-aarch64-low.c b/gdb/gdbserver/linux-aarch64-low.c
index 6066e15..32f153d 100644
--- a/gdb/gdbserver/linux-aarch64-low.c
+++ b/gdb/gdbserver/linux-aarch64-low.c
@@ -267,9 +267,6 @@ aarch64_store_fpregset (struct regcache *regcache, const void *buf)
     supply_register (regcache, AARCH64_V0_REGNO + i, &regset->vregs[i]);
 }
 
-/* Debugging of hardware breakpoint/watchpoint support.  */
-extern int debug_hw_points;
-
 /* Enable miscellaneous debugging output.  The name is historical - it
    was originally used to debug LinuxThreads support.  */
 extern int debug_threads;
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index cf1dffe..b10524f 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -69,9 +69,6 @@ int disable_randomization = 1;
 
 static char **program_argv, **wrapper_argv;
 
-/* Enable debugging of h/w breakpoint/watchpoint support.  */
-int debug_hw_points;
-
 int pass_signals[GDB_SIGNAL_LAST];
 int program_signals[GDB_SIGNAL_LAST];
 int program_signals_p;
diff --git a/gdb/gdbserver/server.h b/gdb/gdbserver/server.h
index e6b2277..97498a6 100644
--- a/gdb/gdbserver/server.h
+++ b/gdb/gdbserver/server.h
@@ -84,7 +84,6 @@ extern ptid_t cont_thread;
 extern ptid_t general_thread;
 
 extern int server_waiting;
-extern int debug_hw_points;
 extern int pass_signals[];
 extern int program_signals[];
 extern int program_signals_p;
diff --git a/gdb/i386-nat.c b/gdb/i386-nat.c
index 499fffb..e5cbd36 100644
--- a/gdb/i386-nat.c
+++ b/gdb/i386-nat.c
@@ -33,9 +33,6 @@
    The functions below implement debug registers sharing by reference
    counts, and allow to watch regions up to 16 bytes long.  */
 
-/* Whether or not to print the mirrored debug registers.  */
-int debug_hw_points;
-
 /* Low-level function vector.  */
 struct i386_dr_low_type i386_dr_low;
 
diff --git a/gdb/nat/i386-dregs.c b/gdb/nat/i386-dregs.c
index 1fa5c19..1e16cf1 100644
--- a/gdb/nat/i386-dregs.c
+++ b/gdb/nat/i386-dregs.c
@@ -175,15 +175,6 @@
 /* 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;
-
-/* Print debugging messages.  */
-#define debug_printf(fmt, args...) \
-  fprintf_unfiltered (gdb_stdlog, fmt, ##args);
-#endif
-
 /* Print the values of the mirrored debug registers.  */
 
 static void
-- 
1.7.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 4/4 v6] Introduce common-debug.h
  2014-08-11 16:17 ` [PATCH 4/4 v6] Introduce common-debug.h Gary Benson
@ 2014-08-12 23:52   ` Doug Evans
  2014-08-20 15:51     ` Pedro Alves
  0 siblings, 1 reply; 7+ messages in thread
From: Doug Evans @ 2014-08-12 23:52 UTC (permalink / raw)
  To: Gary Benson; +Cc: gdb-patches, Pedro Alves

Gary Benson writes:
 > This introduces common-debug.h.  This holds the flag debug_hw_points
 > (which can be set to enable debugging of the hardware breakpoint/
 > watchpoint support code) and debug_printf and debug_vprintf, two
 > functions that the common code can use to print debugging messages.
 > Clients of the common code are expected to implement debug_vprintf;
 > a debug_vprintf function is written from scratch for GDB, and
 > gdbserver's existing debug_printf is repurposed as debug_vprintf.
 > 
 > common/agent.c is changed to use debug_vprintf rather than
 > defining the macro DEBUG_AGENT depending on GDBSERVER.
 > 
 > nat/i386-dregs.c is changed to use the externally-implemented
 > debug_printf, rather than defining it itself, and a now-unnecessary
 > declaration of debug_hw_points is removed.
 > 
 > Various other files are changed to remove declarations of
 > debug_hw_points.
 > 
 > gdb/
 > 2014-08-11  Tom Tromey  <tromey@redhat.com>
 > 	    Gary Benson  <gbenson@redhat.com>
 > 
 > 	* common/common-debug.h: New file.
 > 	* common/common-debug.c: Likewise.
 > 	* debug.c: Likewise.
 > 	* Makefile.in (SFILES): Add common/common-debug.c.
 > 	(HFILES_NO_SRCDIR): Add common/common-debug.h.
 > 	(COMMON_OBS): Add common-debug.o and debug.o.
 > 	(common-debug.o): New rule.
 > 	* common/common-defs.h: Include common-debug.h.
 > 	* common/agent.c (debug_agent_printf): New function.
 > 	(DEBUG_AGENT): Redefine.
 > 	* nat/i386-dregs.c (debug_printf): Undefine.
 > 	(debug_hw_points): Remove.
 > 	* aarch64-linux-nat.c (debug_hw_points): Likewise.
 > 	* i386-nat.c (debug_hw_points): Likewise.
 > 
 > gdb/gdbserver/
 > 2014-08-11  Tom Tromey  <tromey@redhat.com>
 > 	    Gary Benson  <gbenson@redhat.com>
 > 
 > 	* Makefile.in (SFILES): Add common/common-debug.c.
 > 	(OBS): Add common-debug.o.
 > 	(common-debug.o): New rule.
 > 	* debug.h (debug_printf): Don't declare.
 > 	* debug.c (debug_printf): Renamed and rewritten as...
 > 	(debug_vprintf): New function.
 > 	* server.h (debug_hw_points): Remove.
 > 	* server.c (debug_hw_points): Likewise.
 > 	* linux-aarch64-low.c (debug_hw_points): Likewise.

Hi.

I think there's at least still one TODO here.
I don't have a strong preference on how it's solved,
even keeping the current situation (though it feels less preferable),
but I haven't seen it discussed so I want to make sure that the
issue has at least been considered.

i386-nat.c has this:

static void
add_show_debug_regs_command (void)
{
  /* A maintenance command to enable printing the internal DRi mirror
     variables.  */
  add_setshow_boolean_cmd ("show-debug-regs", class_maintenance,
			   &debug_hw_points, _("\
Set whether to show variables that mirror the x86 debug registers."), _("\
Show whether to show variables that mirror the x86 debug registers."), _("\
Use \"on\" to enable, \"off\" to disable.\n\
If enabled, the debug registers values are shown when GDB inserts\n\
or removes a hardware breakpoint or watchpoint, and when the inferior\n\
triggers a breakpoint or watchpoint."),
			   NULL,
			   NULL,
			   &maintenance_set_cmdlist,
			   &maintenance_show_cmdlist);
}

and similarly aarch64-linux-nat.c has this:

static void
add_show_debug_regs_command (void)
{
  /* A maintenance command to enable printing the internal DRi mirror
     variables.  */
  add_setshow_boolean_cmd ("show-debug-regs", class_maintenance,
			   &debug_hw_points, _("\
Set whether to show variables that mirror the AArch64 debug registers."), _("\
Show whether to show variables that mirror the AArch64 debug registers."), _("\
Use \"on\" to enable, \"off\" to disable.\n\
If enabled, the debug registers values are shown when GDB inserts\n\
or removes a hardware breakpoint or watchpoint, and when the inferior\n\
triggers a breakpoint or watchpoint."),
			   NULL,
			   NULL,
			   &maintenance_set_cmdlist,
			   &maintenance_show_cmdlist);
}

If debug_hw_points is now a "common" variable, it seems like
non-architecture-specific code should define the parameter to set/show it.
OTOH, it is nice that the parameter only get defined if/when it's useful.
[I realize being "nat" files these files would never get compiled together
in the same binary.]
OTOOH, it's a debug parameter, I'm less inclined to rigid cleanliness here.
Plus being a "common" parameter means other arches can use it and would be
less inclined to unnecessarily invent a different parameter.

---

Hmmm, I just noticed something: The aarch64 version also uses
add_setshow_boolean_cmd, but there is code like this in aarch64-linux-nat.c:

  if (debug_hw_points > 1)

If we make this a non-arch-specific parameter, we should probably make it
an integer parameter.

---

btw, it's confusing that the variable is named "debug_hw_points"
but the command to set it is "maint set show-debug-regs".  Bleah.
The intuitive naming is to base the variable name off of the parameter name,
but I'm also ok with changing the parameter name.
"set debug hw-points <n>" ?
I don't have a strong opinion, other than if we're making changes
in this area IWBN to clean up the naming while we're at it.
Plus "set debug ..." is more consistent with other such parameters
than "maint set ...".


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 4/4 v6] Introduce common-debug.h
  2014-08-12 23:52   ` Doug Evans
@ 2014-08-20 15:51     ` Pedro Alves
  0 siblings, 0 replies; 7+ messages in thread
From: Pedro Alves @ 2014-08-20 15:51 UTC (permalink / raw)
  To: Doug Evans, Gary Benson; +Cc: gdb-patches

On 08/13/2014 12:52 AM, Doug Evans wrote:
> btw, it's confusing that the variable is named "debug_hw_points"
> but the command to set it is "maint set show-debug-regs".  Bleah.
> The intuitive naming is to base the variable name off of the parameter name,
> but I'm also ok with changing the parameter name.
> "set debug hw-points <n>" ?
> I don't have a strong opinion, other than if we're making changes
> in this area IWBN to clean up the naming while we're at it.
> Plus "set debug ..." is more consistent with other such parameters
> than "maint set ...".

FWIW, if I don't use this command for a while, when I need it again,
I always get confused with the set+show in "maint SET SHOW-debug-regs".

I'd +1 renaming it to "set debug something".

(TBC, I'm not suggesting that doing this now.)

Thanks,
Pedro Alves


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2014-08-20 15:51 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-08-11 15:18 [PATCH 0/4 v6] Common code cleanups (first four) Gary Benson
2014-08-11 15:18 ` [PATCH 1/4 v6] Introduce common/errors.h Gary Benson
2014-08-11 15:18 ` [PATCH 3/4 v6] Move print-utils.h to common-defs.h Gary Benson
2014-08-11 15:18 ` [PATCH 2/4 v6] Introduce common-types.h Gary Benson
2014-08-11 16:17 ` [PATCH 4/4 v6] Introduce common-debug.h Gary Benson
2014-08-12 23:52   ` Doug Evans
2014-08-20 15:51     ` Pedro Alves

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox