Eliminate remaining gdb/guile cleanups

Message ID 20180718233820.9940-1-palves@redhat.com
State New
Headers show
Series
  • Eliminate remaining gdb/guile cleanups
Related show

Commit Message

Pedro Alves July 18, 2018, 11:38 p.m.
The remaining gdb/guile cleanups all handle the memory returned by
gdbscm_scm_to_c_string.

This commit makes gdbscm_scm_to_c_string return a
gdb::unique_xmalloc_ptr instead of a naked pointer, and eliminates the
remaining cleanups.

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

	* guile/guile-internal.h (gdbscm_scm_to_c_string): Now returns a
	gdb::unique_xmalloc_ptr.
	* guile/scm-breakpoint.c (gdbscm_set_breakpoint_condition_x):
	Adjust to use dbscm_wrap and gdb::unique_xmalloc_ptr.
	* guile/scm-exception.c (gdbscm_exception_message_to_string): Use
	copy-initialization.
	* guile/scm-pretty-print.c (ppscm_print_children): Use
	gdb::unique_xmalloc_ptr instead of cleanups.
	(gdbscm_apply_val_pretty_printer): Remove cleanups.
	* guile/scm-string.c (gdbscm_scm_to_c_string): Now returns a
	gdb::unique_xmalloc_ptr.
	* guile/scm-type.c (gdbscm_type_field, gdbscm_type_has_field_p):
	Adjust to use gdb::unique_xmalloc_ptr.
	* guile/scm-utils.c (extract_arg): Adjust.
	* guile/scm-value.c (gdbscm_value_field): Adjust to use
	gdb::unique_xmalloc_ptr instead of a cleanup.
---
 gdb/guile/guile-internal.h   |  2 +-
 gdb/guile/scm-breakpoint.c   | 26 ++++++++----------------
 gdb/guile/scm-exception.c    |  2 +-
 gdb/guile/scm-pretty-print.c | 26 ++++++------------------
 gdb/guile/scm-string.c       |  5 ++---
 gdb/guile/scm-type.c         | 48 +++++++++++++++++++-------------------------
 gdb/guile/scm-utils.c        |  2 +-
 gdb/guile/scm-value.c        | 12 +++--------
 8 files changed, 43 insertions(+), 80 deletions(-)

-- 
2.14.4

Comments

Tom Tromey July 19, 2018, 2:48 p.m. | #1
>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:


Pedro> The remaining gdb/guile cleanups all handle the memory returned by
Pedro> gdbscm_scm_to_c_string.

Pedro> This commit makes gdbscm_scm_to_c_string return a
Pedro> gdb::unique_xmalloc_ptr instead of a naked pointer, and eliminates the
Pedro> remaining cleanups.

I read through this patch and it looked fine to me.
Thanks for doing this.

Tom

Patch

diff --git a/gdb/guile/guile-internal.h b/gdb/guile/guile-internal.h
index 20e002a28e2..89c764b490e 100644
--- a/gdb/guile/guile-internal.h
+++ b/gdb/guile/guile-internal.h
@@ -519,7 +519,7 @@  extern SCM psscm_scm_from_pspace (struct program_space *);
 
 extern int gdbscm_scm_string_to_int (SCM string);
 
-extern char *gdbscm_scm_to_c_string (SCM string);
+extern gdb::unique_xmalloc_ptr<char> gdbscm_scm_to_c_string (SCM string);
 
 extern SCM gdbscm_scm_from_c_string (const char *string);
 
diff --git a/gdb/guile/scm-breakpoint.c b/gdb/guile/scm-breakpoint.c
index 75c24a52354..242fa8022b1 100644
--- a/gdb/guile/scm-breakpoint.c
+++ b/gdb/guile/scm-breakpoint.c
@@ -888,32 +888,22 @@  gdbscm_set_breakpoint_condition_x (SCM self, SCM newvalue)
 {
   breakpoint_smob *bp_smob
     = bpscm_get_valid_breakpoint_smob_arg_unsafe (self, SCM_ARG1, FUNC_NAME);
-  char *exp;
-  struct gdb_exception except = exception_none;
 
   SCM_ASSERT_TYPE (scm_is_string (newvalue) || gdbscm_is_false (newvalue),
 		   newvalue, SCM_ARG2, FUNC_NAME,
 		   _("string or #f"));
 
-  if (gdbscm_is_false (newvalue))
-    exp = NULL;
-  else
-    exp = gdbscm_scm_to_c_string (newvalue);
-
-  TRY
-    {
-      set_breakpoint_condition (bp_smob->bp, exp ? exp : "", 0);
-    }
-  CATCH (ex, RETURN_MASK_ALL)
+  return gdbscm_wrap ([=]
     {
-      except = ex;
-    }
-  END_CATCH
+      gdb::unique_xmalloc_ptr<char> exp
+	= (gdbscm_is_false (newvalue)
+	   ? nullptr
+	   : gdbscm_scm_to_c_string (newvalue));
 
-  xfree (exp);
-  GDBSCM_HANDLE_GDB_EXCEPTION (except);
+      set_breakpoint_condition (bp_smob->bp, exp ? exp.get () : "", 0);
 
-  return SCM_UNSPECIFIED;
+      return SCM_UNSPECIFIED;
+    });
 }
 
 /* (breakpoint-stop <gdb:breakpoint>) -> procedure or #f */
diff --git a/gdb/guile/scm-exception.c b/gdb/guile/scm-exception.c
index f0bcdcd49ef..f4d6fffecca 100644
--- a/gdb/guile/scm-exception.c
+++ b/gdb/guile/scm-exception.c
@@ -599,7 +599,7 @@  gdbscm_exception_message_to_string (SCM exception)
 
   gdbscm_print_exception_message (port, SCM_BOOL_F, key, args);
   gdb::unique_xmalloc_ptr<char> result
-    (gdbscm_scm_to_c_string (scm_get_output_string (port)));
+    = gdbscm_scm_to_c_string (scm_get_output_string (port));
   scm_close_port (port);
   return result;
 }
diff --git a/gdb/guile/scm-pretty-print.c b/gdb/guile/scm-pretty-print.c
index f406c1f8120..3fe36820a6a 100644
--- a/gdb/guile/scm-pretty-print.c
+++ b/gdb/guile/scm-pretty-print.c
@@ -734,7 +734,6 @@  ppscm_print_children (SCM printer, enum display_hint hint,
   unsigned int i;
   SCM children;
   SCM iter = SCM_BOOL_F; /* -Wall */
-  struct cleanup *cleanups;
 
   if (gdbscm_is_false (w_smob->children))
     return;
@@ -746,8 +745,6 @@  ppscm_print_children (SCM printer, enum display_hint hint,
       return;
     }
 
-  cleanups = make_cleanup (null_cleanup, NULL);
-
   /* If we are printing a map or an array, we want special formatting.  */
   is_map = hint == HINT_MAP;
   is_array = hint == HINT_ARRAY;
@@ -788,9 +785,7 @@  ppscm_print_children (SCM printer, enum display_hint hint,
   for (i = 0; i < options->print_max; ++i)
     {
       SCM scm_name, v_scm;
-      char *name;
       SCM item = itscm_safe_call_next_x (iter, gdbscm_memory_error_p);
-      struct cleanup *inner_cleanup = make_cleanup (null_cleanup, NULL);
 
       if (gdbscm_is_exception (item))
 	{
@@ -822,8 +817,8 @@  ppscm_print_children (SCM printer, enum display_hint hint,
 	       " a string"), item);
 	  continue;
 	}
-      name = gdbscm_scm_to_c_string (scm_name);
-      make_cleanup (xfree, name);
+      gdb::unique_xmalloc_ptr<char> name
+	= gdbscm_scm_to_c_string (scm_name);
 
       /* Print initial "{".  For other elements, there are three cases:
 	 1. Maps.  Print a "," after each value element.
@@ -874,7 +869,7 @@  ppscm_print_children (SCM printer, enum display_hint hint,
 	}
       else if (! is_map)
 	{
-	  fputs_filtered (name, stream);
+	  fputs_filtered (name.get (), stream);
 	  fputs_filtered (" = ", stream);
 	}
 
@@ -887,10 +882,9 @@  ppscm_print_children (SCM printer, enum display_hint hint,
 	}
       else if (scm_is_string (v_scm))
 	{
-	  char *output = gdbscm_scm_to_c_string (v_scm);
-
-	  fputs_filtered (output, stream);
-	  xfree (output);
+	  gdb::unique_xmalloc_ptr<char> output
+	    = gdbscm_scm_to_c_string (v_scm);
+	  fputs_filtered (output.get (), stream);
 	}
       else
 	{
@@ -910,8 +904,6 @@  ppscm_print_children (SCM printer, enum display_hint hint,
 
       if (is_map && i % 2 == 0)
 	fputs_filtered ("] = ", stream);
-
-      do_cleanups (inner_cleanup);
     }
 
   if (i)
@@ -934,8 +926,6 @@  ppscm_print_children (SCM printer, enum display_hint hint,
     }
 
  done:
-  do_cleanups (cleanups);
-
   /* Play it safe, make sure ITER doesn't get GC'd.  */
   scm_remember_upto_here_1 (iter);
 }
@@ -957,7 +947,6 @@  gdbscm_apply_val_pretty_printer (const struct extension_language_defn *extlang,
   SCM val_obj = SCM_BOOL_F;
   struct value *value;
   enum display_hint hint;
-  struct cleanup *cleanups;
   enum ext_lang_rc result = EXT_LANG_RC_NOP;
   enum string_repr_result print_result;
   const gdb_byte *valaddr = value_contents_for_printing (val);
@@ -969,8 +958,6 @@  gdbscm_apply_val_pretty_printer (const struct extension_language_defn *extlang,
   if (!gdb_scheme_initialized)
     return EXT_LANG_RC_NOP;
 
-  cleanups = make_cleanup (null_cleanup, NULL);
-
   /* Instantiate the printer.  */
   value = value_from_component (val, type, embedded_offset);
 
@@ -1024,7 +1011,6 @@  gdbscm_apply_val_pretty_printer (const struct extension_language_defn *extlang,
  done:
   if (gdbscm_is_exception (exception))
     ppscm_print_exception_unless_memory_error (exception, stream);
-  do_cleanups (cleanups);
   return result;
 }
 
diff --git a/gdb/guile/scm-string.c b/gdb/guile/scm-string.c
index 56e14c3320c..5779509197f 100644
--- a/gdb/guile/scm-string.c
+++ b/gdb/guile/scm-string.c
@@ -48,13 +48,12 @@  gdbscm_scm_from_c_string (const char *string)
 
 /* Convert an SCM string to a C (latin1) string.
    "latin1" is chosen because Guile won't throw an exception.
-   Space for the result is allocated with malloc, caller must free.
    It is an error to call this if STRING is not a string.  */
 
-char *
+gdb::unique_xmalloc_ptr<char>
 gdbscm_scm_to_c_string (SCM string)
 {
-  return scm_to_latin1_string (string);
+  return gdb::unique_xmalloc_ptr<char> (scm_to_latin1_string (string));
 }
 
 /* Use printf to construct a Scheme string.  */
diff --git a/gdb/guile/scm-type.c b/gdb/guile/scm-type.c
index 5c4490d5ffc..eee0efe9c1d 100644
--- a/gdb/guile/scm-type.c
+++ b/gdb/guile/scm-type.c
@@ -975,8 +975,6 @@  gdbscm_type_field (SCM self, SCM field_scm)
   type_smob *t_smob
     = tyscm_get_type_smob_arg_unsafe (self, SCM_ARG1, FUNC_NAME);
   struct type *type = t_smob->type;
-  char *field;
-  int i;
 
   SCM_ASSERT_TYPE (scm_is_string (field_scm), field_scm, SCM_ARG2, FUNC_NAME,
 		   _("string"));
@@ -990,20 +988,20 @@  gdbscm_type_field (SCM self, SCM field_scm)
     gdbscm_out_of_range_error (FUNC_NAME, SCM_ARG1, self,
 			       _(not_composite_error));
 
-  field = gdbscm_scm_to_c_string (field_scm);
-
-  for (i = 0; i < TYPE_NFIELDS (type); i++)
-    {
-      const char *t_field_name = TYPE_FIELD_NAME (type, i);
+  {
+    gdb::unique_xmalloc_ptr<char> field = gdbscm_scm_to_c_string (field_scm);
 
-      if (t_field_name && (strcmp_iw (t_field_name, field) == 0))
-	{
-	  xfree (field);
-	  return tyscm_make_field_smob (self, i);
-	}
-    }
+    for (int i = 0; i < TYPE_NFIELDS (type); i++)
+      {
+	const char *t_field_name = TYPE_FIELD_NAME (type, i);
 
-  xfree (field);
+	if (t_field_name && (strcmp_iw (t_field_name, field.get ()) == 0))
+	  {
+	    field.reset (nullptr);
+	    return tyscm_make_field_smob (self, i);
+	  }
+      }
+  }
 
   gdbscm_out_of_range_error (FUNC_NAME, SCM_ARG1, field_scm,
 			     _("Unknown field"));
@@ -1018,8 +1016,6 @@  gdbscm_type_has_field_p (SCM self, SCM field_scm)
   type_smob *t_smob
     = tyscm_get_type_smob_arg_unsafe (self, SCM_ARG1, FUNC_NAME);
   struct type *type = t_smob->type;
-  char *field;
-  int i;
 
   SCM_ASSERT_TYPE (scm_is_string (field_scm), field_scm, SCM_ARG2, FUNC_NAME,
 		   _("string"));
@@ -1033,20 +1029,18 @@  gdbscm_type_has_field_p (SCM self, SCM field_scm)
     gdbscm_out_of_range_error (FUNC_NAME, SCM_ARG1, self,
 			       _(not_composite_error));
 
-  field = gdbscm_scm_to_c_string (field_scm);
+  {
+    gdb::unique_xmalloc_ptr<char> field
+      = gdbscm_scm_to_c_string (field_scm);
 
-  for (i = 0; i < TYPE_NFIELDS (type); i++)
-    {
-      const char *t_field_name = TYPE_FIELD_NAME (type, i);
+    for (int i = 0; i < TYPE_NFIELDS (type); i++)
+      {
+	const char *t_field_name = TYPE_FIELD_NAME (type, i);
 
-      if (t_field_name && (strcmp_iw (t_field_name, field) == 0))
-	{
-	  xfree (field);
+	if (t_field_name && (strcmp_iw (t_field_name, field.get ()) == 0))
 	  return SCM_BOOL_T;
-	}
-    }
-
-  xfree (field);
+      }
+  }
 
   return SCM_BOOL_F;
 }
diff --git a/gdb/guile/scm-utils.c b/gdb/guile/scm-utils.c
index 73b0dec73ea..8ea47f9cdf5 100644
--- a/gdb/guile/scm-utils.c
+++ b/gdb/guile/scm-utils.c
@@ -205,7 +205,7 @@  extract_arg (char format_char, SCM arg, void *argp,
 
 	CHECK_TYPE (gdbscm_is_true (scm_string_p (arg)), arg, position,
 		    func_name, _("string"));
-	*arg_ptr = gdbscm_scm_to_c_string (arg);
+	*arg_ptr = gdbscm_scm_to_c_string (arg).release ();
 	break;
       }
     case 't':
diff --git a/gdb/guile/scm-value.c b/gdb/guile/scm-value.c
index 8d42d930bce..43f8001cf08 100644
--- a/gdb/guile/scm-value.c
+++ b/gdb/guile/scm-value.c
@@ -619,20 +619,14 @@  gdbscm_value_field (SCM self, SCM field_scm)
     {
       scoped_value_mark free_values;
 
-      char *field = gdbscm_scm_to_c_string (field_scm);
-
-      struct cleanup *cleanups = make_cleanup (xfree, field);
+      gdb::unique_xmalloc_ptr<char> field = gdbscm_scm_to_c_string (field_scm);
 
       struct value *tmp = v_smob->value;
 
-      struct value *res_val = value_struct_elt (&tmp, NULL, field, NULL,
+      struct value *res_val = value_struct_elt (&tmp, NULL, field.get (), NULL,
 						"struct/class/union");
 
-      SCM result = vlscm_scm_from_value (res_val);
-
-      do_cleanups (cleanups);
-
-      return result;
+      return vlscm_scm_from_value (res_val);
     });
 }