Some gdb_exception{,error,quit} tweaks

Message ID 20190408172425.31039-1-palves@redhat.com
State New
Headers show
Series
  • Some gdb_exception{,error,quit} tweaks
Related show

Commit Message

Pedro Alves April 8, 2019, 5:24 p.m.
- Explicitly include for <string> for std::string.

- Use std::make_shared to construct gdb_exception::message instead of
  operator new, avoiding one heap allocation (2 instead of 3).  Add
  'const char *fmt, va_list ap' parameters to
  gdb_exception{,error,quit}'s ctors, and do the std::make_shared in
  the gdb_exception ctor.

- gdb_exception_error's contructor does not need to have an 'enum
  return_reason' parameter, since always are always RETURN_ERROR, by
  definition.

- Similarly, gdb_exception_quit's contructror does not need to have
  'enum return_reason'/'enum errors' parameters.

- In the gdb_exception_{quit,_error} ctors that take a gdb_exception
  as argument, assert that they're being passed a gdb_exception object
  of the right 'reason'.

gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* common/common-exceptions.c (throw_exception): Don't create
	named object to throw; throw directly.
	(throw_it): Likewise.  Don't initialize gdb_exception::message
	here, with new; pass FMT and AP to the ctor instead.
	* common/common-exceptions.h: Include <string>.
	(gdb_exception::gdb_exception(enum return_reason, enum errors,
	const char *, va_list)): New ctor.  Use std::make_shared.
	(gdb_exception_error::gdb_exception_error(enum return_reason, enum
	errors)): Delete.
	(gdb_exception_error::gdb_exception_error(enum errors, const char
	*, va_list)): New.
	(gdb_exception_error::gdb_exception_error(const gdb_exception &)):
	Add assertion.
	(gdb_exception_quit::gdb_exception_quit(enum return_reason, enum
	errors)): Delete.
	(gdb_exception_quit::gdb_exception_quit(const char *, va_list)): New.
	(gdb_exception_quit::gdb_exception_quit(const gdb_exception &)):
	Add assertion.
---
 gdb/common/common-exceptions.c | 22 ++++------------------
 gdb/common/common-exceptions.h | 19 +++++++++++++++----
 2 files changed, 19 insertions(+), 22 deletions(-)

-- 
2.14.5

Comments

Tom Tromey April 8, 2019, 5:53 p.m. | #1
>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:


Thanks.  This looks good to me.

Pedro> +  gdb_exception (enum return_reason r, enum errors e,
Pedro> +		 const char *fmt, va_list ap)
Pedro> +    : reason (r),
Pedro> +      error (e),
Pedro> +      message (std::make_shared<std::string> (string_vprintf (fmt, ap)))

I think these constructors should probably have ATTRIBUTE_PRINTF with 0
as the "first-to-check" parameter, like string_vprintf itself.

Tom
Pedro Alves April 8, 2019, 7:17 p.m. | #2
On 4/8/19 6:53 PM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

> 

> Thanks.  This looks good to me.

> 

> Pedro> +  gdb_exception (enum return_reason r, enum errors e,

> Pedro> +		 const char *fmt, va_list ap)

> Pedro> +    : reason (r),

> Pedro> +      error (e),

> Pedro> +      message (std::make_shared<std::string> (string_vprintf (fmt, ap)))

> 

> I think these constructors should probably have ATTRIBUTE_PRINTF with 0

> as the "first-to-check" parameter, like string_vprintf itself.


Indeed.  I've added those, fixed a few typos in the commit log, and
pushed it, as below.

From 56be6ea89cdf94078d5dff3734b8c1970dbf52fa Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>

Date: Mon, 8 Apr 2019 13:03:54 +0100
Subject: [PATCH] Some gdb_exception{,error,quit} tweaks

- Explicitly include <string> for std::string.

- Use std::make_shared to construct gdb_exception::message instead of
  operator new, avoiding one heap allocation (2 instead of 3).  Add
  'const char *fmt, va_list ap' parameters to
  gdb_exception{,error,quit}'s ctors, and do the std::make_shared in
  the gdb_exception ctor.

- gdb_exception_error's constructor does not need to have an 'enum
  return_reason' parameter, since it is always RETURN_ERROR, by
  definition.

- Similarly, gdb_exception_quit's contructor does not need to have
  'enum return_reason'/'enum errors' parameters.

- In the gdb_exception_{quit,_error} ctors that take a gdb_exception
  as argument, assert that they're being passed a gdb_exception object
  of the right 'reason'.

gdb/ChangeLog:
2019-04-08  Pedro Alves  <palves@redhat.com>

	* common/common-exceptions.c (throw_exception): Don't create
	named object to throw; throw directly.
	(throw_it): Likewise.  Don't initialize gdb_exception::message
	here, with new; pass FMT and AP to the ctor instead.
	* common/common-exceptions.h: Include <string>.
	(gdb_exception::gdb_exception(enum return_reason, enum errors,
	const char *, va_list)): New ctor.  Use std::make_shared.
	(gdb_exception_error::gdb_exception_error(enum return_reason, enum
	errors)): Delete.
	(gdb_exception_error::gdb_exception_error(enum errors, const char
	*, va_list)): New.
	(gdb_exception_error::gdb_exception_error(const gdb_exception &)):
	Add assertion.
	(gdb_exception_quit::gdb_exception_quit(enum return_reason, enum
	errors)): Delete.
	(gdb_exception_quit::gdb_exception_quit(const char *, va_list)): New.
	(gdb_exception_quit::gdb_exception_quit(const gdb_exception &)):
	Add assertion.
---
 gdb/ChangeLog                  | 21 +++++++++++++++++++++
 gdb/common/common-exceptions.c | 22 ++++------------------
 gdb/common/common-exceptions.h | 22 ++++++++++++++++++----
 3 files changed, 43 insertions(+), 22 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 39c380815aa..9910952aa56 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,24 @@
+2019-04-08  Pedro Alves  <palves@redhat.com>
+
+	* common/common-exceptions.c (throw_exception): Don't create
+	named object to throw; throw directly.
+	(throw_it): Likewise.  Don't initialize gdb_exception::message
+	here, with new; pass FMT and AP to the ctor instead.
+	* common/common-exceptions.h: Include <string>.
+	(gdb_exception::gdb_exception(enum return_reason, enum errors,
+	const char *, va_list)): New ctor.  Use std::make_shared.
+	(gdb_exception_error::gdb_exception_error(enum return_reason, enum
+	errors)): Delete.
+	(gdb_exception_error::gdb_exception_error(enum errors, const char
+	*, va_list)): New.
+	(gdb_exception_error::gdb_exception_error(const gdb_exception &)):
+	Add assertion.
+	(gdb_exception_quit::gdb_exception_quit(enum return_reason, enum
+	errors)): Delete.
+	(gdb_exception_quit::gdb_exception_quit(const char *, va_list)): New.
+	(gdb_exception_quit::gdb_exception_quit(const gdb_exception &)):
+	Add assertion.
+
 2019-04-08  Tom Tromey  <tom@tromey.com>
 
 	* valops.c (value_rtti_indirect_type): Replace throw_exception
diff --git a/gdb/common/common-exceptions.c b/gdb/common/common-exceptions.c
index 83f2c74bfd4..6378dc40d6d 100644
--- a/gdb/common/common-exceptions.c
+++ b/gdb/common/common-exceptions.c
@@ -184,15 +184,9 @@ void
 throw_exception (const gdb_exception &exception)
 {
   if (exception.reason == RETURN_QUIT)
-    {
-      gdb_exception_quit ex (exception);
-      throw ex;
-    }
+    throw gdb_exception_quit (exception);
   else if (exception.reason == RETURN_ERROR)
-    {
-      gdb_exception_error ex (exception);
-      throw ex;
-    }
+    throw gdb_exception_error (exception);
   else
     gdb_assert_not_reached ("invalid return reason");
 }
@@ -202,17 +196,9 @@ throw_it (enum return_reason reason, enum errors error, const char *fmt,
 	  va_list ap)
 {
   if (reason == RETURN_QUIT)
-    {
-      gdb_exception_quit ex (reason, error);
-      ex.message.reset (new std::string (string_vprintf (fmt, ap)));
-      throw ex;
-    }
+    throw gdb_exception_quit (fmt, ap);
   else if (reason == RETURN_ERROR)
-    {
-      gdb_exception_error ex (reason, error);
-      ex.message.reset (new std::string (string_vprintf (fmt, ap)));
-      throw ex;
-    }
+    throw gdb_exception_error (error, fmt, ap);
   else
     gdb_assert_not_reached ("invalid return reason");
 }
diff --git a/gdb/common/common-exceptions.h b/gdb/common/common-exceptions.h
index 1aedb831a62..3f47caec775 100644
--- a/gdb/common/common-exceptions.h
+++ b/gdb/common/common-exceptions.h
@@ -23,6 +23,7 @@
 #include <setjmp.h>
 #include <new>
 #include <memory>
+#include <string>
 
 /* Reasons for calling throw_exceptions().  NOTE: all reason values
    must be different from zero.  enum value 0 is reserved for internal
@@ -123,6 +124,15 @@ struct gdb_exception
   {
   }
 
+  gdb_exception (enum return_reason r, enum errors e,
+		 const char *fmt, va_list ap)
+    ATTRIBUTE_PRINTF (4, 0)
+    : reason (r),
+      error (e),
+      message (std::make_shared<std::string> (string_vprintf (fmt, ap)))
+  {
+  }
+
   /* The copy constructor exists so that we can mark it "noexcept",
      which is a good practice for any sort of exception object.  */
   gdb_exception (const gdb_exception &other) noexcept
@@ -214,27 +224,31 @@ extern int exceptions_state_mc_catch (struct gdb_exception *, int);
 
 struct gdb_exception_error : public gdb_exception
 {
-  gdb_exception_error (enum return_reason r, enum errors e)
-    : gdb_exception (r, e)
+  gdb_exception_error (enum errors e, const char *fmt, va_list ap)
+    ATTRIBUTE_PRINTF (3, 0)
+    : gdb_exception (RETURN_ERROR, e, fmt, ap)
   {
   }
 
   explicit gdb_exception_error (const gdb_exception &ex) noexcept
     : gdb_exception (ex)
   {
+    gdb_assert (ex.reason == RETURN_ERROR);
   }
 };
 
 struct gdb_exception_quit : public gdb_exception
 {
-  gdb_exception_quit (enum return_reason r, enum errors e)
-    : gdb_exception (r, e)
+  gdb_exception_quit (const char *fmt, va_list ap)
+    ATTRIBUTE_PRINTF (2, 0)
+    : gdb_exception (RETURN_QUIT, GDB_NO_ERROR, fmt, ap)
   {
   }
 
   explicit gdb_exception_quit (const gdb_exception &ex) noexcept
     : gdb_exception (ex)
   {
+    gdb_assert (ex.reason == RETURN_QUIT);
   }
 };
 
-- 
2.14.5

Patch

diff --git a/gdb/common/common-exceptions.c b/gdb/common/common-exceptions.c
index 83f2c74bfd4..6378dc40d6d 100644
--- a/gdb/common/common-exceptions.c
+++ b/gdb/common/common-exceptions.c
@@ -184,15 +184,9 @@  void
 throw_exception (const gdb_exception &exception)
 {
   if (exception.reason == RETURN_QUIT)
-    {
-      gdb_exception_quit ex (exception);
-      throw ex;
-    }
+    throw gdb_exception_quit (exception);
   else if (exception.reason == RETURN_ERROR)
-    {
-      gdb_exception_error ex (exception);
-      throw ex;
-    }
+    throw gdb_exception_error (exception);
   else
     gdb_assert_not_reached ("invalid return reason");
 }
@@ -202,17 +196,9 @@  throw_it (enum return_reason reason, enum errors error, const char *fmt,
 	  va_list ap)
 {
   if (reason == RETURN_QUIT)
-    {
-      gdb_exception_quit ex (reason, error);
-      ex.message.reset (new std::string (string_vprintf (fmt, ap)));
-      throw ex;
-    }
+    throw gdb_exception_quit (fmt, ap);
   else if (reason == RETURN_ERROR)
-    {
-      gdb_exception_error ex (reason, error);
-      ex.message.reset (new std::string (string_vprintf (fmt, ap)));
-      throw ex;
-    }
+    throw gdb_exception_error (error, fmt, ap);
   else
     gdb_assert_not_reached ("invalid return reason");
 }
diff --git a/gdb/common/common-exceptions.h b/gdb/common/common-exceptions.h
index 1aedb831a62..8871df6e823 100644
--- a/gdb/common/common-exceptions.h
+++ b/gdb/common/common-exceptions.h
@@ -23,6 +23,7 @@ 
 #include <setjmp.h>
 #include <new>
 #include <memory>
+#include <string>
 
 /* Reasons for calling throw_exceptions().  NOTE: all reason values
    must be different from zero.  enum value 0 is reserved for internal
@@ -123,6 +124,14 @@  struct gdb_exception
   {
   }
 
+  gdb_exception (enum return_reason r, enum errors e,
+		 const char *fmt, va_list ap)
+    : reason (r),
+      error (e),
+      message (std::make_shared<std::string> (string_vprintf (fmt, ap)))
+  {
+  }
+
   /* The copy constructor exists so that we can mark it "noexcept",
      which is a good practice for any sort of exception object.  */
   gdb_exception (const gdb_exception &other) noexcept
@@ -214,27 +223,29 @@  extern int exceptions_state_mc_catch (struct gdb_exception *, int);
 
 struct gdb_exception_error : public gdb_exception
 {
-  gdb_exception_error (enum return_reason r, enum errors e)
-    : gdb_exception (r, e)
+  gdb_exception_error (enum errors e, const char *fmt, va_list ap)
+    : gdb_exception (RETURN_ERROR, e, fmt, ap)
   {
   }
 
   explicit gdb_exception_error (const gdb_exception &ex) noexcept
     : gdb_exception (ex)
   {
+    gdb_assert (ex.reason == RETURN_ERROR);
   }
 };
 
 struct gdb_exception_quit : public gdb_exception
 {
-  gdb_exception_quit (enum return_reason r, enum errors e)
-    : gdb_exception (r, e)
+  gdb_exception_quit (const char *fmt, va_list ap)
+    : gdb_exception (RETURN_QUIT, GDB_NO_ERROR, fmt, ap)
   {
   }
 
   explicit gdb_exception_quit (const gdb_exception &ex) noexcept
     : gdb_exception (ex)
   {
+    gdb_assert (ex.reason == RETURN_QUIT);
   }
 };