[PATCH 01/15 v3] Introduce common/errors.h

[Gary Benson <gbenson@redhat.com>]

Doug Evans wrote:
> fatal is a confusing name IMO.  In gdb it just throws RETURN_QUIT.
> That's not your fault of course, but I wonder if I can trouble you
> to expand on the function comment a bit.  E.g., something like the
> following?
> 
> /* Cause a fatal error.
>    What this does in gdb is throw a RETURN_QUIT exception.  */
> 
> One could expand on the comment, but I didn't want to turn it into
> a FIXME.  [which is really what it should be, but that's a topic
> for another patch :-)]

I have rewritten the documentation on all these functions.  I don't
want to mention details of GDB's and/or gdbserver's implementation
in this file, so what I have done is described what callers should
expect the functions to do.  I hope this is clear enough.

> It's odd to separate out fatal and not vfatal.
> [ditto for the others]
> Can you add vfatal, et.al. to this patch?

I've done that now.  I made the non-v-prefixed functions part of
the common codebase; users of the common codebase are required to
implement the v-prefixed functions only.

Thanks,
Gary

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

	* common/errors.h: New file.
	* common/errors.c: Likewise.
	* Makefile.in (HFILES_NO_SRCDIR) [common/errors.h]: New header.
	(COMMON_OBS) [errors.o]: New object file.
	(errors.o): New rule.
	* utils.h: Include errors.h.
	(perror_with_name, error, verror, fatal, vfatal, warning
	vwarning): Don't declare.
	* common/common-utils.h: Include errors.h.
	(malloc_failure, internal_error): Don't declare.

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

	* Makefile.in (SFILES) [common/errors.c]: New sourcefile.
	(OBS) [errors.o]: New object file.
	(IPA_OBS) [errors-ipa.o]: Likewise.
	(errors.o): New rule.
	(errors-ipa.o): Likewise.
	* utils.h: Include errors.h.
	(perror_with_name, error, fatal, warning): Don't declare.
	* utils.c (warning): Renamed and rewritten as...
	(vwarning): New function.
	(error): Renamed and rewritten as...
	(verror): New function.
	(fatal): Renamed and rewritten as...
	(vfatal): New function.
	(internal_error): Renamed and rewritten as...
	(internal_verror): New function.
---
 gdb/ChangeLog             |   14 ++++++++
 gdb/Makefile.in           |    8 +++-
 gdb/common/common-utils.h |    5 +--
 gdb/common/errors.c       |   71 +++++++++++++++++++++++++++++++++++++++
 gdb/common/errors.h       |   81 +++++++++++++++++++++++++++++++++++++++++++++
 gdb/gdbserver/ChangeLog   |   19 ++++++++++
 gdb/gdbserver/Makefile.in |   16 +++++++--
 gdb/gdbserver/utils.c     |   35 +++++--------------
 gdb/gdbserver/utils.h     |    5 +--
 gdb/utils.c               |   46 -------------------------
 gdb/utils.h               |   22 +------------
 11 files changed, 217 insertions(+), 105 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 ce15501..074ea78 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -935,7 +935,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
+i386-linux-nat.h common/errors.h
 
 # Header files that already have srcdir in them, or which are in objdir.
 
@@ -1034,7 +1034,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 +2144,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-utils.h b/gdb/common/common-utils.h
index 063698d..53be2f8 100644
--- a/gdb/common/common-utils.h
+++ b/gdb/common/common-utils.h
@@ -24,6 +24,7 @@
 #include "ansidecl.h"
 #include <stddef.h>
 #include <stdarg.h>
+#include "errors.h"
 
 /* If possible, define FUNCTION_NAME, a macro containing the name of
    the function being defined.  Since this macro may not always be
@@ -43,10 +44,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..78a6d3e
--- /dev/null
+++ b/gdb/common/errors.c
@@ -0,0 +1,71 @@
+/* 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/>.  */
+
+#include "config.h"
+#include "ansidecl.h"
+#include <stdarg.h>
+#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
+fatal (const char *fmt, ...)
+{
+  va_list ap;
+
+  va_start (ap, fmt);
+  vfatal (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..4e8f4a7
--- /dev/null
+++ b/gdb/common/errors.h
@@ -0,0 +1,81 @@
+/* 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
+
+/* The declarations in this file are, for the time being, separately
+   implemented by gdb and gdbserver.  However they share a common
+   definition so that they can be used by code in common/.  */
+
+/* Issue a warning to the user.  */
+
+extern void warning (const char *fmt, ...) ATTRIBUTE_PRINTF (1, 2);
+
+extern void vwarning (const char *fmt, va_list args)
+     ATTRIBUTE_PRINTF (1, 0);
+
+/* Throw an error.  The current operation will be aborted.  The
+   message will be issued to the user.  The application will
+   return to a state where it is accepting commands from the user.  */
+
+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);
+
+/* Throw an error, creating the message by combining STRING with the
+   system error message for errno.  The current operation will be
+   aborted.  The message will be issued to the user.  The application
+   will return to a state where it is accepting commands from the
+   user.  */
+
+extern void perror_with_name (const char *string) ATTRIBUTE_NORETURN;
+
+/* Throw a fatal error.  The current operation will be aborted.  The
+   message will be issued to the user.  The application will exit.  */
+
+extern void fatal (const char *fmt, ...)
+     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 2);
+
+extern void vfatal (const char *fmt, va_list args)
+     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 0);
+
+/* Throw an internal error.  This should be used to indicate
+   programming errors such as assertion failures, as opposed
+   to more general errors the user may encounter.  The current
+   operation will be aborted.  The message will be issued to
+   the user.  The application may offer the user the choice to
+   return to the command level, or the application may exit.  */
+
+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);
+
+/* Special case internal error used to handle memory allocation
+   failures.  */
+
+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 2d0b331..6c666e1 100644
--- a/gdb/gdbserver/utils.c
+++ b/gdb/gdbserver/utils.c
@@ -77,20 +77,17 @@ 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 command level.  */
 
 void
-error (const char *string,...)
+verror (const char *fmt, 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);
+  vfprintf (stderr, fmt, args);
   fprintf (stderr, "\n");
 #ifndef IN_PROCESS_AGENT
   longjmp (toplevel, 1);
@@ -99,48 +96,36 @@ error (const char *string,...)
 #endif
 }
 
-/* Print an error message and exit reporting failure.
-   This is for a error that we cannot continue from.
-   STRING and ARG are passed to fprintf.  */
+/* Print an error message and exit reporting failure.  */
 
 /* VARARGS */
 void
-fatal (const char *string,...)
+vfatal (const char *fmt, va_list args)
 {
-  va_list args;
-  va_start (args, string);
   fprintf (stderr, PREFIX);
-  vfprintf (stderr, string, args);
+  vfprintf (stderr, fmt, args);
   fprintf (stderr, "\n");
-  va_end (args);
   exit (1);
 }
 
 /* VARARGS */
 void
-warning (const char *string,...)
+vwarning (const char *fmt, va_list args)
 {
-  va_list args;
-  va_start (args, string);
   fprintf (stderr, PREFIX);
-  vfprintf (stderr, string, args);
+  vfprintf (stderr, fmt, 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..819fa35 100644
--- a/gdb/gdbserver/utils.h
+++ b/gdb/gdbserver/utils.h
@@ -20,11 +20,8 @@
 #define UTILS_H
 
 #include "print-utils.h"
+#include "errors.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 d324227..b2f9f0f 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -533,22 +533,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.  */
@@ -559,16 +543,6 @@ verror (const char *string, va_list args)
   throw_verror (GENERIC_ERROR, string, args);
 }
 
-void
-error (const char *string, ...)
-{
-  va_list args;
-
-  va_start (args, string);
-  throw_verror (GENERIC_ERROR, string, args);
-  va_end (args);
-}
-
 /* Print an error message and quit.
    The first argument STRING is the error message, used as a fprintf string,
    and the remaining args are passed as arguments to it.  */
@@ -580,16 +554,6 @@ vfatal (const char *string, va_list args)
 }
 
 void
-fatal (const char *string, ...)
-{
-  va_list args;
-
-  va_start (args, string);
-  throw_vfatal (string, args);
-  va_end (args);
-}
-
-void
 error_stream (struct ui_file *stream)
 {
   char *message = ui_file_xstrdup (stream, NULL);
@@ -836,16 +800,6 @@ internal_verror (const char *file, int line, const char *fmt, va_list ap)
   fatal (_("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 a91f551..e48fc43 100644
--- a/gdb/utils.h
+++ b/gdb/utils.h
@@ -24,6 +24,7 @@
 #include "cleanups.h"
 #include "exceptions.h"
 #include "print-utils.h"
+#include "errors.h"
 
 extern void initialize_utils (void);
 
@@ -269,7 +270,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,24 +283,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 vfatal (const char *fmt, va_list ap)
-     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 0);
-
-extern void fatal (const char *fmt, ...)
-     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 2);
-
-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);
@@ -308,10 +292,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);