[RFA,2/4] Change gdbscm_exception_message_to_string to return a unique_xmalloc_ptr

Message ID 20180527152009.4228-3-tom@tromey.com
State New, archived
Headers

Commit Message

Tom Tromey May 27, 2018, 3:20 p.m. UTC
  This changes gdbscm_exception_message_to_string to return a
unique_xmalloc_ptr, allowing for the removal of some cleanups.
unique_xmalloc_ptr was chosen because at the root of the call chains
is a function from Guile that returns a malloc'd string.

ChangeLog
2018-05-26  Tom Tromey  <tom@tromey.com>

	* guile/scm-param.c (pascm_signal_setshow_error): Update.
	* guile/guile-internal.h (gdbscm_exception_message_to_string):
	Update.
	* guile/scm-cmd.c (cmdscm_function): Update.
	* guile/scm-pretty-print.c
	(ppscm_print_exception_unless_memory_error): Update.
	* guile/scm-exception.c (gdbscm_exception_message_to_string):
	Return unique_xmalloc_ptr.
---
 gdb/ChangeLog                | 11 +++++++++++
 gdb/guile/guile-internal.h   |  3 ++-
 gdb/guile/scm-cmd.c          |  6 +++---
 gdb/guile/scm-exception.c    | 11 ++++-------
 gdb/guile/scm-param.c        |  6 +++---
 gdb/guile/scm-pretty-print.c | 17 ++++++++---------
 6 files changed, 31 insertions(+), 23 deletions(-)
  

Comments

Pedro Alves July 17, 2018, 1:08 p.m. UTC | #1
On 05/27/2018 04:20 PM, Tom Tromey wrote:

> ChangeLog
> 2018-05-26  Tom Tromey  <tom@tromey.com>
> 
> 	* guile/scm-param.c (pascm_signal_setshow_error): Update.
> 	* guile/guile-internal.h (gdbscm_exception_message_to_string):
> 	Update.
> 	* guile/scm-cmd.c (cmdscm_function): Update.
> 	* guile/scm-pretty-print.c
> 	(ppscm_print_exception_unless_memory_error): Update.
> 	* guile/scm-exception.c (gdbscm_exception_message_to_string):
> 	Return unique_xmalloc_ptr.

OK.

> @@ -601,9 +598,9 @@ gdbscm_exception_message_to_string (SCM exception)
>      }
>  
>    gdbscm_print_exception_message (port, SCM_BOOL_F, key, args);
> -  result = gdbscm_scm_to_c_string (scm_get_output_string (port));
> +  gdb::unique_xmalloc_ptr<char> result
> +    (gdbscm_scm_to_c_string (scm_get_output_string (port)));
>    scm_close_port (port);
> -
>    return result;
>  }

Did you try making gdbscm_scm_to_c_string return a unique_ptr too?

Thanks,
Pedro Alves
  
Tom Tromey July 17, 2018, 7:10 p.m. UTC | #2
>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

>> gdbscm_print_exception_message (port, SCM_BOOL_F, key, args);
>> -  result = gdbscm_scm_to_c_string (scm_get_output_string (port));
>> +  gdb::unique_xmalloc_ptr<char> result
>> +    (gdbscm_scm_to_c_string (scm_get_output_string (port)));
>> scm_close_port (port);
>> -
>> return result;
>> }

Pedro> Did you try making gdbscm_scm_to_c_string return a unique_ptr too?

Yes, but this runs into the use of
GDBSCM_HANDLE_GDB_EXCEPTION_WITH_CLEANUPS in (at least) gdbscm_value_field.
I haven't looked into what to do about this.

Tom
  

Patch

diff --git a/gdb/guile/guile-internal.h b/gdb/guile/guile-internal.h
index 6bce58e150..20e2c70e16 100644
--- a/gdb/guile/guile-internal.h
+++ b/gdb/guile/guile-internal.h
@@ -362,7 +362,8 @@  extern void gdbscm_print_exception_with_stack (SCM port, SCM stack,
 
 extern void gdbscm_print_gdb_exception (SCM port, SCM exception);
 
-extern char *gdbscm_exception_message_to_string (SCM exception);
+extern gdb::unique_xmalloc_ptr<char> gdbscm_exception_message_to_string
+    (SCM exception);
 
 extern excp_matcher_func gdbscm_memory_error_p;
 
diff --git a/gdb/guile/scm-cmd.c b/gdb/guile/scm-cmd.c
index 64243d1ba2..8bb46622a9 100644
--- a/gdb/guile/scm-cmd.c
+++ b/gdb/guile/scm-cmd.c
@@ -316,10 +316,10 @@  cmdscm_function (struct cmd_list_element *command,
 	 itself.  */
       if (gdbscm_user_error_p (gdbscm_exception_key (result)))
 	{
-	  char *msg = gdbscm_exception_message_to_string (result);
+	  gdb::unique_xmalloc_ptr<char> msg
+	    = gdbscm_exception_message_to_string (result);
 
-	  make_cleanup (xfree, msg);
-	  error ("%s", msg);
+	  error ("%s", msg.get ());
 	}
       else
 	{
diff --git a/gdb/guile/scm-exception.c b/gdb/guile/scm-exception.c
index e4b81a1fd1..f0bcdcd49e 100644
--- a/gdb/guile/scm-exception.c
+++ b/gdb/guile/scm-exception.c
@@ -575,16 +575,13 @@  gdbscm_print_gdb_exception (SCM port, SCM exception)
 
 /* Return a string description of <gdb:exception> EXCEPTION.
    If EXCEPTION is a gdb:with-stack exception, unwrap it, a backtrace
-   is never returned as part of the result.
+   is never returned as part of the result.  */
 
-   Space for the result is malloc'd, the caller must free.  */
-
-char *
+gdb::unique_xmalloc_ptr<char>
 gdbscm_exception_message_to_string (SCM exception)
 {
   SCM port = scm_open_output_string ();
   SCM key, args;
-  char *result;
 
   gdb_assert (gdbscm_is_exception (exception));
 
@@ -601,9 +598,9 @@  gdbscm_exception_message_to_string (SCM exception)
     }
 
   gdbscm_print_exception_message (port, SCM_BOOL_F, key, args);
-  result = gdbscm_scm_to_c_string (scm_get_output_string (port));
+  gdb::unique_xmalloc_ptr<char> result
+    (gdbscm_scm_to_c_string (scm_get_output_string (port)));
   scm_close_port (port);
-
   return result;
 }
 
diff --git a/gdb/guile/scm-param.c b/gdb/guile/scm-param.c
index d48f14e55c..7ff4af9501 100644
--- a/gdb/guile/scm-param.c
+++ b/gdb/guile/scm-param.c
@@ -251,10 +251,10 @@  pascm_signal_setshow_error (SCM exception, const char *msg)
      itself.  */
   if (gdbscm_user_error_p (gdbscm_exception_key (exception)))
     {
-      char *excp_text = gdbscm_exception_message_to_string (exception);
+      gdb::unique_xmalloc_ptr<char> excp_text
+	= gdbscm_exception_message_to_string (exception);
 
-      make_cleanup (xfree, excp_text);
-      error ("%s", excp_text);
+      error ("%s", excp_text.get ());
     }
   else
     {
diff --git a/gdb/guile/scm-pretty-print.c b/gdb/guile/scm-pretty-print.c
index 5e8a2a998d..eea524b104 100644
--- a/gdb/guile/scm-pretty-print.c
+++ b/gdb/guile/scm-pretty-print.c
@@ -614,25 +614,24 @@  ppscm_print_exception_unless_memory_error (SCM exception,
 {
   if (gdbscm_memory_error_p (gdbscm_exception_key (exception)))
     {
-      char *msg = gdbscm_exception_message_to_string (exception);
-      struct cleanup *cleanup = make_cleanup (xfree, msg);
+      gdb::unique_xmalloc_ptr<char> msg
+	= gdbscm_exception_message_to_string (exception);
 
       /* This "shouldn't happen", but play it safe.  */
-      if (msg == NULL || *msg == '\0')
+      if (msg == NULL || msg.get ()[0] == '\0')
 	fprintf_filtered (stream, _("<error reading variable>"));
       else
 	{
 	  /* Remove the trailing newline.  We could instead call a special
 	     routine for printing memory error messages, but this is easy
 	     enough for now.  */
-	  size_t len = strlen (msg);
+	  char *msg_text = msg.get ();
+	  size_t len = strlen (msg_text);
 
-	  if (msg[len - 1] == '\n')
-	    msg[len - 1] = '\0';
-	  fprintf_filtered (stream, _("<error reading variable: %s>"), msg);
+	  if (msg_text[len - 1] == '\n')
+	    msg_text[len - 1] = '\0';
+	  fprintf_filtered (stream, _("<error reading variable: %s>"), msg_text);
 	}
-
-      do_cleanups (cleanup);
     }
   else
     gdbscm_print_gdb_exception (SCM_BOOL_F, exception);