public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [RFA 2/5] Introduce ui_file_up and use it to remove cleanups
  2017-01-15 13:43 [RFA 0/5] more cleanup removal in Python Tom Tromey
                   ` (2 preceding siblings ...)
  2017-01-15 13:43 ` [RFA 1/5] Remove some ui_out-related cleanups from Python Tom Tromey
@ 2017-01-15 13:43 ` Tom Tromey
  2017-01-16  9:59   ` Trevor Saunders
  2017-01-16 17:58   ` Pedro Alves
  2017-01-15 13:43 ` [RFA 5/5] Remove some gotos from Python Tom Tromey
  4 siblings, 2 replies; 30+ messages in thread
From: Tom Tromey @ 2017-01-15 13:43 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This introduces a new ui_file_up typedef, which is a specialization of
unique_ptr that calls ui_file_delete.  This patch also changes
mem_fileopen to return a ui_file_up, and fixes the users.  It also
updates a few other spots in the Python code to use this rather than
cleanups.

If at some point ui_file_delete is removed in favor of a destructor, I
think the typedef could be changed and the default deletion policy
used instead.

2017-01-15  Tom Tromey  <tom@tromey.com>

	* ui-file.h (mem_fileopen): Return ui_file_up.
	(ui_file_up): New typedef.
	(ui_file_deleter): New struct.
	* ui-file.c (mem_fileopen): Update.
	* mi/mi-out.c (mi_out_new): Update.
	* arm-tdep.c (_initialize_arm_tdep): Update.
	* gdbarch.c: Rebuild.
	* gdbarch.sh (get_gdbarch): Use ui_file_up.
	* compile/compile-c-support.c (c_compute_program): Use
	ui_file_up.
	* guile/scm-value.c (vlscm_print_value_smob): Use ui_file_up.
	(gdbscm_value_print): Use ui_file_up.
	* guile/scm-type.c (tyscm_type_name): Use ui_file_up.
	* guile/scm-frame.c (frscm_print_frame_smob): Use ui_file_up.
	* guile/scm-disasm.c (gdbscm_arch_disassemble): Use ui_file_up.
	* guile/scm-breakpoint.c (gdbscm_breakpoint_commands): Use
	ui_file_up.
	* mi/mi-console.c (mi_console_file_new): Update.
	* record-btrace.c (btrace_insn_history): Use ui_file_up.
	* language.c (add_language): Use ui_file_up.
	* infrun.c (print_target_wait_results): Use ui_file_up.
	* dwarf2read.c (dwarf2_compute_name): Use ui_file_up.
	* cp-support.c (inspect_type): Use ui_file_up.
	(replace_typedefs_qualified_name): Use ui_file_up.
	* tracepoint.c (append_exp): Use ui_file_up.
	* aix-thread.c (aix_thread_extra_thread_info): Use ui_file_up.
	* c-exp.y (oper): Use ui_file_up.
	* python/py-unwind.c (unwind_infopy_str): Use ui_file_up.
	* python/py-frame.c (frapy_str): Use ui_file_up.
	* python/py-arch.c (archpy_disassemble): Use ui_file_up.
	* python/py-value.c (valpy_str): Use ui_file_up.
	* python/py-type.c (typy_str): Use ui_file_up.
	* python/py-framefilter.c (py_print_type, py_print_value)
	(py_print_single_arg): Use ui_file_up.
	* python/py-breakpoint.c (bppy_get_commands): Use ui_file_up.
	* mi/mi-main.c (mi_cmd_data_evaluate_expression): Use ui_file_up.
	(output_register, mi_cmd_data_read_memory, mi_cmd_execute)
	(print_variable_or_computed): Likewise.
	* compile/compile.c (compile_to_object): Use ui_file_up.
	* compile/compile-c-symbols.c (generate_c_for_for_one_variable):
	Use ui_file_up.
	* cli/cli-setshow.c (do_show_command): Use ui_file_up.
	* xtensa-tdep.c (xtensa_verify_config): Use ui_file_up.
	* varobj.c (varobj_value_get_print_value): Use ui_file_up.
	* typeprint.c (type_to_string): Use ui_file_up.
	* top.c (execute_command_to_string, quit_confirm): Use
	ui_file_up.
	* stack.c (print_frame_args): Don't call mem_fileopen.
	(print_frame_arg): Use ui_file_up.
	(print_frame): Use ui_file_up.
	* rust-lang.c (rust_get_disr_info): Use ui_file_up.
	* remote.c (escape_buffer): Use ui_file_up.
	* reggroups.c (maintenance_print_reggroups): Use ui_file_up.
	* regcache.c (regcache_print): Use ui_file_up.
	* printcmd.c (eval_command): Use ui_file_up.
	* maint.c (maintenance_print_architecture): Use ui_file_up.
	* location.c (explicit_to_string_internal): Use ui_file_up.
	* infcmd.c (print_return_value_1): Use ui_file_up.
	* dummy-frame.c (maintenance_print_dummy_frames): Use ui_file_up.
	* disasm.c (gdb_pretty_print_insn, gdb_disassembly): Use
	ui_file_up.
	* breakpoint.c (update_inserted_breakpoint_locations)
	(insert_breakpoint_locations, reattach_breakpoints)
	(print_breakpoint_location, print_one_detail_ranged_breakpoint):
	Use ui_file_up.
	(print_it_watchpoint): Likewise.
	* ada-varobj.c (ada_varobj_scalar_image)
	(ada_varobj_get_value_image): Use ui_file_up.
	* ada-valprint.c (ada_print_floating): Use ui_file_up.
	* ada-lang.c (type_as_string): Use ui_file_up.
---
 gdb/ChangeLog                   | 73 +++++++++++++++++++++++++++++++++++++++
 gdb/ada-lang.c                  | 11 ++----
 gdb/ada-valprint.c              |  9 ++---
 gdb/ada-varobj.c                | 19 ++++-------
 gdb/aix-thread.c                | 18 +++++-----
 gdb/arm-tdep.c                  |  2 +-
 gdb/breakpoint.c                | 76 +++++++++++++++++------------------------
 gdb/c-exp.y                     |  7 ++--
 gdb/cli/cli-setshow.c           | 34 ++++++++----------
 gdb/compile/compile-c-support.c | 44 ++++++++++++------------
 gdb/compile/compile-c-symbols.c | 20 +++++------
 gdb/compile/compile.c           |  9 +++--
 gdb/cp-support.c                | 51 +++++++++++++--------------
 gdb/disasm.c                    | 24 ++++++-------
 gdb/dummy-frame.c               |  9 ++---
 gdb/dwarf2read.c                | 35 +++++++++----------
 gdb/gdbarch.c                   | 28 +++++++--------
 gdb/gdbarch.sh                  | 16 ++++-----
 gdb/guile/scm-breakpoint.c      | 11 ++----
 gdb/guile/scm-disasm.c          | 12 +++----
 gdb/guile/scm-frame.c           |  9 +++--
 gdb/guile/scm-type.c            | 13 ++-----
 gdb/guile/scm-value.c           | 18 ++++------
 gdb/infcmd.c                    | 10 ++----
 gdb/infrun.c                    | 15 ++++----
 gdb/language.c                  | 10 +++---
 gdb/location.c                  | 30 +++++++---------
 gdb/maint.c                     |  7 ++--
 gdb/mi/mi-cmd-stack.c           | 21 ++++++------
 gdb/mi/mi-console.c             |  2 +-
 gdb/mi/mi-main.c                | 75 +++++++++++++++++-----------------------
 gdb/mi/mi-out.c                 |  2 +-
 gdb/printcmd.c                  |  9 ++---
 gdb/python/py-arch.c            | 27 ++++-----------
 gdb/python/py-breakpoint.c      | 11 ++----
 gdb/python/py-frame.c           |  7 ++--
 gdb/python/py-framefilter.c     | 39 +++++++--------------
 gdb/python/py-type.c            | 11 ++----
 gdb/python/py-unwind.c          | 21 ++++++------
 gdb/python/py-value.c           |  9 ++---
 gdb/record-btrace.c             | 11 +++---
 gdb/regcache.c                  |  7 ++--
 gdb/reggroups.c                 |  7 ++--
 gdb/remote.c                    | 10 ++----
 gdb/rust-lang.c                 | 10 ++----
 gdb/stack.c                     | 39 ++++++++-------------
 gdb/top.c                       | 35 +++++++------------
 gdb/tracepoint.c                |  7 ++--
 gdb/typeprint.c                 | 11 ++----
 gdb/ui-file.c                   |  4 +--
 gdb/ui-file.h                   | 18 ++++++++--
 gdb/varobj.c                    | 28 +++++----------
 gdb/xtensa-tdep.c               | 22 +++++-------
 53 files changed, 466 insertions(+), 597 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 8a2848f..08670fb 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,78 @@
 2017-01-15  Tom Tromey  <tom@tromey.com>
 
+	* ui-file.h (mem_fileopen): Return ui_file_up.
+	(ui_file_up): New typedef.
+	(ui_file_deleter): New struct.
+	* ui-file.c (mem_fileopen): Update.
+	* mi/mi-out.c (mi_out_new): Update.
+	* arm-tdep.c (_initialize_arm_tdep): Update.
+	* gdbarch.c: Rebuild.
+	* gdbarch.sh (get_gdbarch): Use ui_file_up.
+	* compile/compile-c-support.c (c_compute_program): Use
+	ui_file_up.
+	* guile/scm-value.c (vlscm_print_value_smob): Use ui_file_up.
+	(gdbscm_value_print): Use ui_file_up.
+	* guile/scm-type.c (tyscm_type_name): Use ui_file_up.
+	* guile/scm-frame.c (frscm_print_frame_smob): Use ui_file_up.
+	* guile/scm-disasm.c (gdbscm_arch_disassemble): Use ui_file_up.
+	* guile/scm-breakpoint.c (gdbscm_breakpoint_commands): Use
+	ui_file_up.
+	* mi/mi-console.c (mi_console_file_new): Update.
+	* record-btrace.c (btrace_insn_history): Use ui_file_up.
+	* language.c (add_language): Use ui_file_up.
+	* infrun.c (print_target_wait_results): Use ui_file_up.
+	* dwarf2read.c (dwarf2_compute_name): Use ui_file_up.
+	* cp-support.c (inspect_type): Use ui_file_up.
+	(replace_typedefs_qualified_name): Use ui_file_up.
+	* tracepoint.c (append_exp): Use ui_file_up.
+	* aix-thread.c (aix_thread_extra_thread_info): Use ui_file_up.
+	* c-exp.y (oper): Use ui_file_up.
+	* python/py-unwind.c (unwind_infopy_str): Use ui_file_up.
+	* python/py-frame.c (frapy_str): Use ui_file_up.
+	* python/py-arch.c (archpy_disassemble): Use ui_file_up.
+	* python/py-value.c (valpy_str): Use ui_file_up.
+	* python/py-type.c (typy_str): Use ui_file_up.
+	* python/py-framefilter.c (py_print_type, py_print_value)
+	(py_print_single_arg): Use ui_file_up.
+	* python/py-breakpoint.c (bppy_get_commands): Use ui_file_up.
+	* mi/mi-main.c (mi_cmd_data_evaluate_expression): Use ui_file_up.
+	(output_register, mi_cmd_data_read_memory, mi_cmd_execute)
+	(print_variable_or_computed): Likewise.
+	* compile/compile.c (compile_to_object): Use ui_file_up.
+	* compile/compile-c-symbols.c (generate_c_for_for_one_variable):
+	Use ui_file_up.
+	* cli/cli-setshow.c (do_show_command): Use ui_file_up.
+	* xtensa-tdep.c (xtensa_verify_config): Use ui_file_up.
+	* varobj.c (varobj_value_get_print_value): Use ui_file_up.
+	* typeprint.c (type_to_string): Use ui_file_up.
+	* top.c (execute_command_to_string, quit_confirm): Use
+	ui_file_up.
+	* stack.c (print_frame_args): Don't call mem_fileopen.
+	(print_frame_arg): Use ui_file_up.
+	(print_frame): Use ui_file_up.
+	* rust-lang.c (rust_get_disr_info): Use ui_file_up.
+	* remote.c (escape_buffer): Use ui_file_up.
+	* reggroups.c (maintenance_print_reggroups): Use ui_file_up.
+	* regcache.c (regcache_print): Use ui_file_up.
+	* printcmd.c (eval_command): Use ui_file_up.
+	* maint.c (maintenance_print_architecture): Use ui_file_up.
+	* location.c (explicit_to_string_internal): Use ui_file_up.
+	* infcmd.c (print_return_value_1): Use ui_file_up.
+	* dummy-frame.c (maintenance_print_dummy_frames): Use ui_file_up.
+	* disasm.c (gdb_pretty_print_insn, gdb_disassembly): Use
+	ui_file_up.
+	* breakpoint.c (update_inserted_breakpoint_locations)
+	(insert_breakpoint_locations, reattach_breakpoints)
+	(print_breakpoint_location, print_one_detail_ranged_breakpoint):
+	Use ui_file_up.
+	(print_it_watchpoint): Likewise.
+	* ada-varobj.c (ada_varobj_scalar_image)
+	(ada_varobj_get_value_image): Use ui_file_up.
+	* ada-valprint.c (ada_print_floating): Use ui_file_up.
+	* ada-lang.c (type_as_string): Use ui_file_up.
+
+2017-01-15  Tom Tromey  <tom@tromey.com>
+
 	* ui-out.h (ui_out_emit_type): New class.
 	(ui_out_emit_tuple, ui_out_emit_list): New typedefs.
 	* python/py-framefilter.c (py_print_single_arg): Use gdb::optional
diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 381752b..e809ae3 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -7602,16 +7602,11 @@ ada_value_struct_elt (struct value *arg, char *name, int no_err)
 static std::string
 type_as_string (struct type *type)
 {
-  struct ui_file *tmp_stream = mem_fileopen ();
-  struct cleanup *old_chain;
-
-  tmp_stream = mem_fileopen ();
-  old_chain = make_cleanup_ui_file_delete (tmp_stream);
+  ui_file_up tmp_stream = mem_fileopen ();
 
-  type_print (type, "", tmp_stream, -1);
-  std::string str = ui_file_as_string (tmp_stream);
+  type_print (type, "", tmp_stream.get (), -1);
+  std::string str = ui_file_as_string (tmp_stream.get ());
 
-  do_cleanups (old_chain);
   return str;
 }
 
diff --git a/gdb/ada-valprint.c b/gdb/ada-valprint.c
index 0a9e325..a62ba1c 100644
--- a/gdb/ada-valprint.c
+++ b/gdb/ada-valprint.c
@@ -298,12 +298,11 @@ static void
 ada_print_floating (const gdb_byte *valaddr, struct type *type,
 		    struct ui_file *stream)
 {
-  struct ui_file *tmp_stream = mem_fileopen ();
-  struct cleanup *cleanups = make_cleanup_ui_file_delete (tmp_stream);
+  ui_file_up tmp_stream = mem_fileopen ();
 
-  print_floating (valaddr, type, tmp_stream);
+  print_floating (valaddr, type, tmp_stream.get ());
 
-  std::string s = ui_file_as_string (tmp_stream);
+  std::string s = ui_file_as_string (tmp_stream.get ());
   size_t skip_count = 0;
 
   /* Modify for Ada rules.  */
@@ -342,8 +341,6 @@ ada_print_floating (const gdb_byte *valaddr, struct type *type,
     }
   else
     fprintf_filtered (stream, "%s", &s[skip_count]);
-
-  do_cleanups (cleanups);
 }
 
 void
diff --git a/gdb/ada-varobj.c b/gdb/ada-varobj.c
index 52e3247..ba46a14 100644
--- a/gdb/ada-varobj.c
+++ b/gdb/ada-varobj.c
@@ -79,12 +79,10 @@ ada_varobj_decode_var (struct value **value_ptr, struct type **type_ptr)
 static std::string
 ada_varobj_scalar_image (struct type *type, LONGEST val)
 {
-  struct ui_file *buf = mem_fileopen ();
-  struct cleanup *cleanups = make_cleanup_ui_file_delete (buf);
+  ui_file_up buf = mem_fileopen ();
 
-  ada_print_scalar (type, val, buf);
-  std::string result = ui_file_as_string (buf);
-  do_cleanups (cleanups);
+  ada_print_scalar (type, val, buf.get ());
+  std::string result = ui_file_as_string (buf.get ());
 
   return result;
 }
@@ -808,16 +806,11 @@ static std::string
 ada_varobj_get_value_image (struct value *value,
 			    struct value_print_options *opts)
 {
-  struct ui_file *buffer;
-  struct cleanup *old_chain;
+  ui_file_up buffer = mem_fileopen ();
 
-  buffer = mem_fileopen ();
-  old_chain = make_cleanup_ui_file_delete (buffer);
+  common_val_print (value, buffer.get (), 0, opts, current_language);
+  std::string result = ui_file_as_string (buffer.get ());
 
-  common_val_print (value, buffer, 0, opts, current_language);
-  std::string result = ui_file_as_string (buffer);
-
-  do_cleanups (old_chain);
   return result;
 }
 
diff --git a/gdb/aix-thread.c b/gdb/aix-thread.c
index ea64220..23c537b 100644
--- a/gdb/aix-thread.c
+++ b/gdb/aix-thread.c
@@ -1749,7 +1749,6 @@ static char *
 aix_thread_extra_thread_info (struct target_ops *self,
 			      struct thread_info *thread)
 {
-  struct ui_file *buf;
   int status;
   pthdb_pthread_t pdtid;
   pthdb_tid_t tid;
@@ -1762,43 +1761,42 @@ aix_thread_extra_thread_info (struct target_ops *self,
   if (!PD_TID (thread->ptid))
     return NULL;
 
-  buf = mem_fileopen ();
+  ui_file_up buf = mem_fileopen ();
 
   pdtid = thread->priv->pdtid;
   tid = thread->priv->tid;
 
   if (tid != PTHDB_INVALID_TID)
     /* i18n: Like "thread-identifier %d, [state] running, suspended" */
-    fprintf_unfiltered (buf, _("tid %d"), (int)tid);
+    fprintf_unfiltered (buf.get (), _("tid %d"), (int)tid);
 
   status = pthdb_pthread_state (pd_session, pdtid, &state);
   if (status != PTHDB_SUCCESS)
     state = PST_NOTSUP;
-  fprintf_unfiltered (buf, ", %s", state2str (state));
+  fprintf_unfiltered (buf.get (), ", %s", state2str (state));
 
   status = pthdb_pthread_suspendstate (pd_session, pdtid, 
 				       &suspendstate);
   if (status == PTHDB_SUCCESS && suspendstate == PSS_SUSPENDED)
     /* i18n: Like "Thread-Id %d, [state] running, suspended" */
-    fprintf_unfiltered (buf, _(", suspended"));
+    fprintf_unfiltered (buf.get (), _(", suspended"));
 
   status = pthdb_pthread_detachstate (pd_session, pdtid, 
 				      &detachstate);
   if (status == PTHDB_SUCCESS && detachstate == PDS_DETACHED)
     /* i18n: Like "Thread-Id %d, [state] running, detached" */
-    fprintf_unfiltered (buf, _(", detached"));
+    fprintf_unfiltered (buf.get (), _(", detached"));
 
   pthdb_pthread_cancelpend (pd_session, pdtid, &cancelpend);
   if (status == PTHDB_SUCCESS && cancelpend)
     /* i18n: Like "Thread-Id %d, [state] running, cancel pending" */
-    fprintf_unfiltered (buf, _(", cancel pending"));
+    fprintf_unfiltered (buf.get (), _(", cancel pending"));
 
-  ui_file_write (buf, "", 1);
+  ui_file_write (buf.get (), "", 1);
 
   xfree (ret);			/* Free old buffer.  */
 
-  ret = ui_file_xstrdup (buf, NULL);
-  ui_file_delete (buf);
+  ret = ui_file_xstrdup (buf.get (), NULL);
 
   return ret;
 }
diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index 2bdfa57..91df61c 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -9645,7 +9645,7 @@ _initialize_arm_tdep (void)
   valid_disassembly_styles[num_disassembly_options] = NULL;
 
   /* Create the help text.  */
-  stb = mem_fileopen ();
+  stb = mem_fileopen ().release ();
   fprintf_unfiltered (stb, "%s%s%s",
 		      _("The valid values are:\n"),
 		      regdesc,
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 867dbb9..caf3af6 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -3061,12 +3061,11 @@ update_inserted_breakpoint_locations (void)
   int hw_breakpoint_error = 0;
   int hw_bp_details_reported = 0;
 
-  struct ui_file *tmp_error_stream = mem_fileopen ();
-  struct cleanup *cleanups = make_cleanup_ui_file_delete (tmp_error_stream);
+  ui_file_up tmp_error_stream = mem_fileopen ();
 
   /* Explicitly mark the warning -- this will only be printed if
      there was an error.  */
-  fprintf_unfiltered (tmp_error_stream, "Warning:\n");
+  fprintf_unfiltered (tmp_error_stream.get (), "Warning:\n");
 
   save_current_space_and_thread ();
 
@@ -3093,7 +3092,7 @@ update_inserted_breakpoint_locations (void)
 	  && ptid_equal (inferior_ptid, null_ptid))
 	continue;
 
-      val = insert_bp_location (bl, tmp_error_stream, &disabled_breaks,
+      val = insert_bp_location (bl, tmp_error_stream.get (), &disabled_breaks,
 				    &hw_breakpoint_error, &hw_bp_details_reported);
       if (val)
 	error_flag = val;
@@ -3102,10 +3101,8 @@ update_inserted_breakpoint_locations (void)
   if (error_flag)
     {
       target_terminal_ours_for_output ();
-      error_stream (tmp_error_stream);
+      error_stream (tmp_error_stream.get ());
     }
-
-  do_cleanups (cleanups);
 }
 
 /* Used when starting or continuing the program.  */
@@ -3121,12 +3118,11 @@ insert_breakpoint_locations (void)
   int hw_breakpoint_error = 0;
   int hw_bp_error_explained_already = 0;
 
-  struct ui_file *tmp_error_stream = mem_fileopen ();
-  struct cleanup *cleanups = make_cleanup_ui_file_delete (tmp_error_stream);
+  ui_file_up tmp_error_stream = mem_fileopen ();
   
   /* Explicitly mark the warning -- this will only be printed if
      there was an error.  */
-  fprintf_unfiltered (tmp_error_stream, "Warning:\n");
+  fprintf_unfiltered (tmp_error_stream.get (), "Warning:\n");
 
   save_current_space_and_thread ();
 
@@ -3152,7 +3148,7 @@ insert_breakpoint_locations (void)
 	  && ptid_equal (inferior_ptid, null_ptid))
 	continue;
 
-      val = insert_bp_location (bl, tmp_error_stream, &disabled_breaks,
+      val = insert_bp_location (bl, tmp_error_stream.get (), &disabled_breaks,
 				    &hw_breakpoint_error, &hw_bp_error_explained_already);
       if (val)
 	error_flag = val;
@@ -3187,7 +3183,7 @@ insert_breakpoint_locations (void)
 	      remove_breakpoint (loc);
 
 	  hw_breakpoint_error = 1;
-	  fprintf_unfiltered (tmp_error_stream,
+	  fprintf_unfiltered (tmp_error_stream.get (),
 			      "Could not insert hardware watchpoint %d.\n", 
 			      bpt->number);
 	  error_flag = -1;
@@ -3200,15 +3196,13 @@ insert_breakpoint_locations (void)
          message about possibly exhausted resources.  */
       if (hw_breakpoint_error && !hw_bp_error_explained_already)
 	{
-	  fprintf_unfiltered (tmp_error_stream, 
+	  fprintf_unfiltered (tmp_error_stream.get (), 
 			      "Could not insert hardware breakpoints:\n\
 You may have requested too many hardware breakpoints/watchpoints.\n");
 	}
       target_terminal_ours_for_output ();
-      error_stream (tmp_error_stream);
+      error_stream (tmp_error_stream.get ());
     }
-
-  do_cleanups (cleanups);
 }
 
 /* Used when the program stops.
@@ -3283,7 +3277,6 @@ reattach_breakpoints (int pid)
   struct cleanup *old_chain;
   struct bp_location *bl, **blp_tmp;
   int val;
-  struct ui_file *tmp_error_stream;
   int dummy1 = 0, dummy2 = 0, dummy3 = 0;
   struct inferior *inf;
   struct thread_info *tp;
@@ -3297,8 +3290,7 @@ reattach_breakpoints (int pid)
 
   inferior_ptid = tp->ptid;
 
-  tmp_error_stream = mem_fileopen ();
-  make_cleanup_ui_file_delete (tmp_error_stream);
+  ui_file_up tmp_error_stream = mem_fileopen ();
 
   ALL_BP_LOCATIONS (bl, blp_tmp)
   {
@@ -3308,7 +3300,8 @@ reattach_breakpoints (int pid)
     if (bl->inserted)
       {
 	bl->inserted = 0;
-	val = insert_bp_location (bl, tmp_error_stream, &dummy1, &dummy2, &dummy3);
+	val = insert_bp_location (bl, tmp_error_stream.get (),
+				  &dummy1, &dummy2, &dummy3);
 	if (val != 0)
 	  {
 	    do_cleanups (old_chain);
@@ -6179,14 +6172,11 @@ print_breakpoint_location (struct breakpoint *b,
     }
   else if (loc)
     {
-      struct ui_file *stb = mem_fileopen ();
-      struct cleanup *stb_chain = make_cleanup_ui_file_delete (stb);
+      ui_file_up stb = mem_fileopen ();
 
-      print_address_symbolic (loc->gdbarch, loc->address, stb,
+      print_address_symbolic (loc->gdbarch, loc->address, stb.get (),
 			      demangle, "");
-      uiout->field_stream ("at", stb);
-
-      do_cleanups (stb_chain);
+      uiout->field_stream ("at", stb.get ());
     }
   else
     {
@@ -10293,8 +10283,7 @@ print_one_detail_ranged_breakpoint (const struct breakpoint *b,
 {
   CORE_ADDR address_start, address_end;
   struct bp_location *bl = b->loc;
-  struct ui_file *stb = mem_fileopen ();
-  struct cleanup *cleanup = make_cleanup_ui_file_delete (stb);
+  ui_file_up stb = mem_fileopen ();
 
   gdb_assert (bl);
 
@@ -10302,13 +10291,11 @@ print_one_detail_ranged_breakpoint (const struct breakpoint *b,
   address_end = address_start + bl->length - 1;
 
   uiout->text ("\taddress range: ");
-  fprintf_unfiltered (stb, "[%s, %s]",
+  fprintf_unfiltered (stb.get (), "[%s, %s]",
 		      print_core_address (bl->gdbarch, address_start),
 		      print_core_address (bl->gdbarch, address_end));
-  uiout->field_stream ("addr", stb);
+  uiout->field_stream ("addr", stb.get ());
   uiout->text ("\n");
-
-  do_cleanups (cleanup);
 }
 
 /* Implement the "print_mention" breakpoint_ops method for
@@ -10740,7 +10727,6 @@ print_it_watchpoint (bpstat bs)
 {
   struct cleanup *old_chain;
   struct breakpoint *b;
-  struct ui_file *stb;
   enum print_stop_action result;
   struct watchpoint *w;
   struct ui_out *uiout = current_uiout;
@@ -10750,8 +10736,8 @@ print_it_watchpoint (bpstat bs)
   b = bs->breakpoint_at;
   w = (struct watchpoint *) b;
 
-  stb = mem_fileopen ();
-  old_chain = make_cleanup_ui_file_delete (stb);
+  ui_file_up stb = mem_fileopen ();
+  old_chain = make_cleanup (null_cleanup, NULL);
 
   annotate_watchpoint (b->number);
   maybe_print_thread_hit_breakpoint (uiout);
@@ -10766,11 +10752,11 @@ print_it_watchpoint (bpstat bs)
       mention (b);
       make_cleanup_ui_out_tuple_begin_end (uiout, "value");
       uiout->text ("\nOld value = ");
-      watchpoint_value_print (bs->old_val, stb);
-      uiout->field_stream ("old", stb);
+      watchpoint_value_print (bs->old_val, stb.get ());
+      uiout->field_stream ("old", stb.get ());
       uiout->text ("\nNew value = ");
-      watchpoint_value_print (w->val, stb);
-      uiout->field_stream ("new", stb);
+      watchpoint_value_print (w->val, stb.get ());
+      uiout->field_stream ("new", stb.get ());
       uiout->text ("\n");
       /* More than one watchpoint may have been triggered.  */
       result = PRINT_UNKNOWN;
@@ -10783,8 +10769,8 @@ print_it_watchpoint (bpstat bs)
       mention (b);
       make_cleanup_ui_out_tuple_begin_end (uiout, "value");
       uiout->text ("\nValue = ");
-      watchpoint_value_print (w->val, stb);
-      uiout->field_stream ("value", stb);
+      watchpoint_value_print (w->val, stb.get ());
+      uiout->field_stream ("value", stb.get ());
       uiout->text ("\n");
       result = PRINT_UNKNOWN;
       break;
@@ -10799,8 +10785,8 @@ print_it_watchpoint (bpstat bs)
 	  mention (b);
 	  make_cleanup_ui_out_tuple_begin_end (uiout, "value");
 	  uiout->text ("\nOld value = ");
-	  watchpoint_value_print (bs->old_val, stb);
-	  uiout->field_stream ("old", stb);
+	  watchpoint_value_print (bs->old_val, stb.get ());
+	  uiout->field_stream ("old", stb.get ());
 	  uiout->text ("\nNew value = ");
 	}
       else
@@ -10813,8 +10799,8 @@ print_it_watchpoint (bpstat bs)
 	  make_cleanup_ui_out_tuple_begin_end (uiout, "value");
 	  uiout->text ("\nValue = ");
 	}
-      watchpoint_value_print (w->val, stb);
-      uiout->field_stream ("new", stb);
+      watchpoint_value_print (w->val, stb.get ());
+      uiout->field_stream ("new", stb.get ());
       uiout->text ("\n");
       result = PRINT_UNKNOWN;
       break;
diff --git a/gdb/c-exp.y b/gdb/c-exp.y
index 8a92cce..2f618f3 100644
--- a/gdb/c-exp.y
+++ b/gdb/c-exp.y
@@ -1555,12 +1555,11 @@ oper:	OPERATOR NEW
 	|	OPERATOR OBJC_LBRAC ']'
 			{ $$ = operator_stoken ("[]"); }
 	|	OPERATOR conversion_type_id
-			{ struct ui_file *buf = mem_fileopen ();
+			{ ui_file_up buf = mem_fileopen ();
 
-			  c_print_type ($2, NULL, buf, -1, 0,
+			  c_print_type ($2, NULL, buf.get (), -1, 0,
 					&type_print_raw_options);
-			  std::string name = ui_file_as_string (buf);
-			  ui_file_delete (buf);
+			  std::string name = ui_file_as_string (buf.get ());
 			  $$ = operator_stoken (name.c_str ());
 			}
 	;
diff --git a/gdb/cli/cli-setshow.c b/gdb/cli/cli-setshow.c
index 9298665..f098b7b 100644
--- a/gdb/cli/cli-setshow.c
+++ b/gdb/cli/cli-setshow.c
@@ -568,13 +568,10 @@ void
 do_show_command (const char *arg, int from_tty, struct cmd_list_element *c)
 {
   struct ui_out *uiout = current_uiout;
-  struct cleanup *old_chain;
-  struct ui_file *stb;
 
   gdb_assert (c->type == show_cmd);
 
-  stb = mem_fileopen ();
-  old_chain = make_cleanup_ui_file_delete (stb);
+  ui_file_up stb = mem_fileopen ();
 
   /* Possibly call the pre hook.  */
   if (c->pre_show_hook)
@@ -584,29 +581,29 @@ do_show_command (const char *arg, int from_tty, struct cmd_list_element *c)
     {
     case var_string:
       if (*(char **) c->var)
-	fputstr_filtered (*(char **) c->var, '"', stb);
+	fputstr_filtered (*(char **) c->var, '"', stb.get ());
       break;
     case var_string_noescape:
     case var_optional_filename:
     case var_filename:
     case var_enum:
       if (*(char **) c->var)
-	fputs_filtered (*(char **) c->var, stb);
+	fputs_filtered (*(char **) c->var, stb.get ());
       break;
     case var_boolean:
-      fputs_filtered (*(int *) c->var ? "on" : "off", stb);
+      fputs_filtered (*(int *) c->var ? "on" : "off", stb.get ());
       break;
     case var_auto_boolean:
       switch (*(enum auto_boolean*) c->var)
 	{
 	case AUTO_BOOLEAN_TRUE:
-	  fputs_filtered ("on", stb);
+	  fputs_filtered ("on", stb.get ());
 	  break;
 	case AUTO_BOOLEAN_FALSE:
-	  fputs_filtered ("off", stb);
+	  fputs_filtered ("off", stb.get ());
 	  break;
 	case AUTO_BOOLEAN_AUTO:
-	  fputs_filtered ("auto", stb);
+	  fputs_filtered ("auto", stb.get ());
 	  break;
 	default:
 	  internal_error (__FILE__, __LINE__,
@@ -619,24 +616,24 @@ do_show_command (const char *arg, int from_tty, struct cmd_list_element *c)
     case var_zuinteger:
       if (c->var_type == var_uinteger
 	  && *(unsigned int *) c->var == UINT_MAX)
-	fputs_filtered ("unlimited", stb);
+	fputs_filtered ("unlimited", stb.get ());
       else
-	fprintf_filtered (stb, "%u", *(unsigned int *) c->var);
+	fprintf_filtered (stb.get (), "%u", *(unsigned int *) c->var);
       break;
     case var_integer:
     case var_zinteger:
       if (c->var_type == var_integer
 	  && *(int *) c->var == INT_MAX)
-	fputs_filtered ("unlimited", stb);
+	fputs_filtered ("unlimited", stb.get ());
       else
-	fprintf_filtered (stb, "%d", *(int *) c->var);
+	fprintf_filtered (stb.get (), "%d", *(int *) c->var);
       break;
     case var_zuinteger_unlimited:
       {
 	if (*(int *) c->var == -1)
-	  fputs_filtered ("unlimited", stb);
+	  fputs_filtered ("unlimited", stb.get ());
 	else
-	  fprintf_filtered (stb, "%d", *(int *) c->var);
+	  fprintf_filtered (stb.get (), "%d", *(int *) c->var);
       }
       break;
     default:
@@ -650,17 +647,16 @@ do_show_command (const char *arg, int from_tty, struct cmd_list_element *c)
      MI and CLI specific versions.  */
 
   if (uiout->is_mi_like_p ())
-    uiout->field_stream ("value", stb);
+    uiout->field_stream ("value", stb.get ());
   else
     {
-      std::string value = ui_file_as_string (stb);
+      std::string value = ui_file_as_string (stb.get ());
 
       if (c->show_value_func != NULL)
 	c->show_value_func (gdb_stdout, from_tty, c, value.c_str ());
       else
 	deprecated_show_value_hack (gdb_stdout, from_tty, c, value.c_str ());
     }
-  do_cleanups (old_chain);
 
   c->func (c, NULL, from_tty);
 }
diff --git a/gdb/compile/compile-c-support.c b/gdb/compile/compile-c-support.c
index 877bfb7..a467dbf 100644
--- a/gdb/compile/compile-c-support.c
+++ b/gdb/compile/compile-c-support.c
@@ -333,15 +333,15 @@ c_compute_program (struct compile_instance *inst,
 		   const struct block *expr_block,
 		   CORE_ADDR expr_pc)
 {
-  struct ui_file *buf, *var_stream = NULL;
+  ui_file_up var_stream;
   std::string code;
   struct cleanup *cleanup;
   struct compile_c_instance *context = (struct compile_c_instance *) inst;
 
-  buf = mem_fileopen ();
-  cleanup = make_cleanup_ui_file_delete (buf);
+  ui_file_up buf = mem_fileopen ();
+  cleanup = make_cleanup (null_cleanup, NULL);
 
-  write_macro_definitions (expr_block, expr_pc, buf);
+  write_macro_definitions (expr_block, expr_pc, buf.get ());
 
   /* Do not generate local variable information for "raw"
      compilations.  In this case we aren't emitting our own function
@@ -356,20 +356,20 @@ c_compute_program (struct compile_instance *inst,
 	 register struct before the function body.  This requires a
 	 temporary stream.  */
       var_stream = mem_fileopen ();
-      make_cleanup_ui_file_delete (var_stream);
       registers_used = generate_c_for_variable_locations (context,
-							  var_stream, gdbarch,
+							  var_stream.get (),
+							  gdbarch,
 							  expr_block, expr_pc);
       make_cleanup (xfree, registers_used);
 
       fputs_unfiltered ("typedef unsigned int"
 			" __attribute__ ((__mode__(__pointer__)))"
 			" __gdb_uintptr;\n",
-			buf);
+			buf.get ());
       fputs_unfiltered ("typedef int"
 			" __attribute__ ((__mode__(__pointer__)))"
 			" __gdb_intptr;\n",
-			buf);
+			buf.get ());
 
       /* Iterate all log2 sizes in bytes supported by c_get_mode_for_size.  */
       for (i = 0; i < 4; ++i)
@@ -377,24 +377,24 @@ c_compute_program (struct compile_instance *inst,
 	  const char *mode = c_get_mode_for_size (1 << i);
 
 	  gdb_assert (mode != NULL);
-	  fprintf_unfiltered (buf,
+	  fprintf_unfiltered (buf.get (),
 			      "typedef int"
 			      " __attribute__ ((__mode__(__%s__)))"
 			      " __gdb_int_%s;\n",
 			      mode, mode);
 	}
 
-      generate_register_struct (buf, gdbarch, registers_used);
+      generate_register_struct (buf.get (), gdbarch, registers_used);
     }
 
-  add_code_header (inst->scope, buf);
+  add_code_header (inst->scope, buf.get ());
 
   if (inst->scope == COMPILE_I_SIMPLE_SCOPE
       || inst->scope == COMPILE_I_PRINT_ADDRESS_SCOPE
       || inst->scope == COMPILE_I_PRINT_VALUE_SCOPE)
     {
-      ui_file_put (var_stream, ui_file_write_for_put, buf);
-      fputs_unfiltered ("#pragma GCC user_expression\n", buf);
+      ui_file_put (var_stream.get (), ui_file_write_for_put, buf.get ());
+      fputs_unfiltered ("#pragma GCC user_expression\n", buf.get ());
     }
 
   /* The user expression has to be in its own scope, so that "extern"
@@ -402,15 +402,15 @@ c_compute_program (struct compile_instance *inst,
      declaration is in the same scope as the declaration provided by
      gdb.  */
   if (inst->scope != COMPILE_I_RAW_SCOPE)
-    fputs_unfiltered ("{\n", buf);
+    fputs_unfiltered ("{\n", buf.get ());
 
-  fputs_unfiltered ("#line 1 \"gdb command line\"\n", buf);
+  fputs_unfiltered ("#line 1 \"gdb command line\"\n", buf.get ());
 
   switch (inst->scope)
     {
     case COMPILE_I_PRINT_ADDRESS_SCOPE:
     case COMPILE_I_PRINT_VALUE_SCOPE:
-      fprintf_unfiltered (buf,
+      fprintf_unfiltered (buf.get (),
 "__auto_type " COMPILE_I_EXPR_VAL " = %s;\n"
 "typeof (%s) *" COMPILE_I_EXPR_PTR_TYPE ";\n"
 "memcpy (" COMPILE_I_PRINT_OUT_ARG ", %s" COMPILE_I_EXPR_VAL ",\n"
@@ -420,22 +420,22 @@ c_compute_program (struct compile_instance *inst,
 			   ? "&" : ""));
       break;
     default:
-      fputs_unfiltered (input, buf);
+      fputs_unfiltered (input, buf.get ());
       break;
     }
 
-  fputs_unfiltered ("\n", buf);
+  fputs_unfiltered ("\n", buf.get ());
 
   /* For larger user expressions the automatic semicolons may be
      confusing.  */
   if (strchr (input, '\n') == NULL)
-    fputs_unfiltered (";\n", buf);
+    fputs_unfiltered (";\n", buf.get ());
 
   if (inst->scope != COMPILE_I_RAW_SCOPE)
-    fputs_unfiltered ("}\n", buf);
+    fputs_unfiltered ("}\n", buf.get ());
 
-  add_code_footer (inst->scope, buf);
-  code = ui_file_as_string (buf);
+  add_code_footer (inst->scope, buf.get ());
+  code = ui_file_as_string (buf.get ());
   do_cleanups (cleanup);
   return code;
 }
diff --git a/gdb/compile/compile-c-symbols.c b/gdb/compile/compile-c-symbols.c
index 6010006..1a21684 100644
--- a/gdb/compile/compile-c-symbols.c
+++ b/gdb/compile/compile-c-symbols.c
@@ -651,14 +651,11 @@ generate_c_for_for_one_variable (struct compile_c_instance *compiler,
     {
       if (is_dynamic_type (SYMBOL_TYPE (sym)))
 	{
-	  struct ui_file *size_file = mem_fileopen ();
-	  struct cleanup *cleanup = make_cleanup_ui_file_delete (size_file);
+	  ui_file_up size_file = mem_fileopen ();
 
-	  generate_vla_size (compiler, size_file, gdbarch, registers_used, pc,
-			     SYMBOL_TYPE (sym), sym);
-	  ui_file_put (size_file, ui_file_write_for_put, stream);
-
-	  do_cleanups (cleanup);
+	  generate_vla_size (compiler, size_file.get (), gdbarch,
+			     registers_used, pc, SYMBOL_TYPE (sym), sym);
+	  ui_file_put (size_file.get (), ui_file_write_for_put, stream);
 	}
 
       if (SYMBOL_COMPUTED_OPS (sym) != NULL)
@@ -667,15 +664,14 @@ generate_c_for_for_one_variable (struct compile_c_instance *compiler,
 	  struct cleanup *cleanup = make_cleanup (xfree, generated_name);
 	  /* We need to emit to a temporary buffer in case an error
 	     occurs in the middle.  */
-	  struct ui_file *local_file = mem_fileopen ();
+	  ui_file_up local_file = mem_fileopen ();
 
-	  make_cleanup_ui_file_delete (local_file);
-	  SYMBOL_COMPUTED_OPS (sym)->generate_c_location (sym, local_file,
+	  SYMBOL_COMPUTED_OPS (sym)->generate_c_location (sym,
+							  local_file.get (),
 							  gdbarch,
 							  registers_used,
 							  pc, generated_name);
-	  ui_file_put (local_file, ui_file_write_for_put, stream);
-
+	  ui_file_put (local_file.get (), ui_file_write_for_put, stream);
 	  do_cleanups (cleanup);
 	}
       else
diff --git a/gdb/compile/compile.c b/gdb/compile/compile.c
index 0ae2125..3c21d46 100644
--- a/gdb/compile/compile.c
+++ b/gdb/compile/compile.c
@@ -499,17 +499,16 @@ compile_to_object (struct command_line *cmd, const char *cmd_string,
 
   if (cmd != NULL)
     {
-      struct ui_file *stream = mem_fileopen ();
+      ui_file_up stream = mem_fileopen ();
       struct command_line *iter;
 
-      make_cleanup_ui_file_delete (stream);
       for (iter = cmd->body_list[0]; iter; iter = iter->next)
 	{
-	  fputs_unfiltered (iter->line, stream);
-	  fputs_unfiltered ("\n", stream);
+	  fputs_unfiltered (iter->line, stream.get ());
+	  fputs_unfiltered ("\n", stream.get ());
 	}
 
-      input_buf = ui_file_as_string (stream);
+      input_buf = ui_file_as_string (stream.get ());
       input = input_buf.c_str ();
     }
   else if (cmd_string != NULL)
diff --git a/gdb/cp-support.c b/gdb/cp-support.c
index f4498f1..dbfca23 100644
--- a/gdb/cp-support.c
+++ b/gdb/cp-support.c
@@ -192,7 +192,6 @@ inspect_type (struct demangle_parse_info *info,
 	  int is_anon;
 	  struct type *type;
 	  std::unique_ptr<demangle_parse_info> i;
-	  struct ui_file *buf;
 
 	  /* Get the real type of the typedef.  */
 	  type = check_typedef (otype);
@@ -228,23 +227,23 @@ inspect_type (struct demangle_parse_info *info,
 		type = last;
 	    }
 
-	  buf = mem_fileopen ();
-	  TRY
 	  {
-	    type_print (type, "", buf, -1);
-	  }
-
-	  /* If type_print threw an exception, there is little point
-	     in continuing, so just bow out gracefully.  */
-	  CATCH (except, RETURN_MASK_ERROR)
-	    {
-	      ui_file_delete (buf);
-	      return 0;
-	    }
-	  END_CATCH
+	    ui_file_up buf = mem_fileopen ();
+	    TRY
+	      {
+		type_print (type, "", buf.get (), -1);
+	      }
+
+	    /* If type_print threw an exception, there is little point
+	       in continuing, so just bow out gracefully.  */
+	    CATCH (except, RETURN_MASK_ERROR)
+	      {
+		return 0;
+	      }
+	    END_CATCH
 
-	  name = ui_file_obsavestring (buf, &info->obstack, &len);
-	  ui_file_delete (buf);
+	      name = ui_file_obsavestring (buf.get (), &info->obstack, &len);
+	  }
 
 	  /* Turn the result into a new tree.  Note that this
 	     tree will contain pointers into NAME, so NAME cannot
@@ -301,7 +300,7 @@ replace_typedefs_qualified_name (struct demangle_parse_info *info,
 {
   long len;
   char *name;
-  struct ui_file *buf = mem_fileopen ();
+  ui_file_up buf = mem_fileopen ();
   struct demangle_component *comp = ret_comp;
 
   /* Walk each node of the qualified name, reconstructing the name of
@@ -315,9 +314,9 @@ replace_typedefs_qualified_name (struct demangle_parse_info *info,
 	{
 	  struct demangle_component newobj;
 
-	  ui_file_write (buf, d_left (comp)->u.s_name.s,
+	  ui_file_write (buf.get (), d_left (comp)->u.s_name.s,
 			 d_left (comp)->u.s_name.len);
-	  name = ui_file_obsavestring (buf, &info->obstack, &len);
+	  name = ui_file_obsavestring (buf.get (), &info->obstack, &len);
 	  newobj.type = DEMANGLE_COMPONENT_NAME;
 	  newobj.u.s_name.s = name;
 	  newobj.u.s_name.len = len;
@@ -330,12 +329,11 @@ replace_typedefs_qualified_name (struct demangle_parse_info *info,
 		 string and replace the top DEMANGLE_COMPONENT_QUAL_NAME
 		 node.  */
 
-	      ui_file_rewind (buf);
+	      ui_file_rewind (buf.get ());
 	      n = cp_comp_to_string (&newobj, 100);
 	      if (n == NULL)
 		{
 		  /* If something went astray, abort typedef substitutions.  */
-		  ui_file_delete (buf);
 		  return;
 		}
 
@@ -360,14 +358,13 @@ replace_typedefs_qualified_name (struct demangle_parse_info *info,
 	  if (name == NULL)
 	    {
 	      /* If something went astray, abort typedef substitutions.  */
-	      ui_file_delete (buf);
 	      return;
 	    }
-	  fputs_unfiltered (name, buf);
+	  fputs_unfiltered (name, buf.get ());
 	  xfree (name);
 	}
 
-      ui_file_write (buf, "::", 2);
+      ui_file_write (buf.get (), "::", 2);
       comp = d_right (comp);
     }
 
@@ -377,8 +374,8 @@ replace_typedefs_qualified_name (struct demangle_parse_info *info,
 
   if (comp->type == DEMANGLE_COMPONENT_NAME)
     {
-      ui_file_write (buf, comp->u.s_name.s, comp->u.s_name.len);
-      name = ui_file_obsavestring (buf, &info->obstack, &len);
+      ui_file_write (buf.get (), comp->u.s_name.s, comp->u.s_name.len);
+      name = ui_file_obsavestring (buf.get (), &info->obstack, &len);
 
       /* Replace the top (DEMANGLE_COMPONENT_QUAL_NAME) node
 	 with a DEMANGLE_COMPONENT_NAME node containing the whole
@@ -390,8 +387,6 @@ replace_typedefs_qualified_name (struct demangle_parse_info *info,
     }
   else
     replace_typedefs (info, comp, finder, data);
-
-  ui_file_delete (buf);
 }
 
 
diff --git a/gdb/disasm.c b/gdb/disasm.c
index f419501..f64b00e 100644
--- a/gdb/disasm.c
+++ b/gdb/disasm.c
@@ -250,9 +250,7 @@ gdb_pretty_print_insn (struct gdbarch *gdbarch, struct ui_out *uiout,
 
       /* Build the opcodes using a temporary stream so we can
 	 write them out in a single go for the MI.  */
-      struct ui_file *opcode_stream = mem_fileopen ();
-      struct cleanup *cleanups =
-	make_cleanup_ui_file_delete (opcode_stream);
+      ui_file_up opcode_stream = mem_fileopen ();
 
       size = gdbarch_print_insn (gdbarch, pc, di);
       end_pc = pc + size;
@@ -262,15 +260,13 @@ gdb_pretty_print_insn (struct gdbarch *gdbarch, struct ui_out *uiout,
 	  err = (*di->read_memory_func) (pc, &data, 1, di);
 	  if (err != 0)
 	    (*di->memory_error_func) (err, pc, di);
-	  fprintf_filtered (opcode_stream, "%s%02x",
+	  fprintf_filtered (opcode_stream.get (), "%s%02x",
 			    spacer, (unsigned) data);
 	  spacer = " ";
 	}
 
-      uiout->field_stream ("opcodes", opcode_stream);
+      uiout->field_stream ("opcodes", opcode_stream.get ());
       uiout->text ("\t");
-
-      do_cleanups (cleanups);
     }
   else
     size = gdbarch_print_insn (gdbarch, pc, di);
@@ -787,9 +783,8 @@ gdb_disassembly (struct gdbarch *gdbarch, struct ui_out *uiout,
 		 char *file_string, int flags, int how_many,
 		 CORE_ADDR low, CORE_ADDR high)
 {
-  struct ui_file *stb = mem_fileopen ();
-  struct cleanup *cleanups = make_cleanup_ui_file_delete (stb);
-  struct disassemble_info di = gdb_disassemble_info (gdbarch, stb);
+  ui_file_up stb = mem_fileopen ();
+  struct disassemble_info di = gdb_disassemble_info (gdbarch, stb.get ());
   struct symtab *symtab;
   int nlines = -1;
 
@@ -801,17 +796,18 @@ gdb_disassembly (struct gdbarch *gdbarch, struct ui_out *uiout,
 
   if (!(flags & (DISASSEMBLY_SOURCE_DEPRECATED | DISASSEMBLY_SOURCE))
       || nlines <= 0)
-    do_assembly_only (gdbarch, uiout, &di, low, high, how_many, flags, stb);
+    do_assembly_only (gdbarch, uiout, &di, low, high, how_many, flags,
+		      stb.get ());
 
   else if (flags & DISASSEMBLY_SOURCE)
     do_mixed_source_and_assembly (gdbarch, uiout, &di, symtab, low, high,
-				  how_many, flags, stb);
+				  how_many, flags, stb.get ());
 
   else if (flags & DISASSEMBLY_SOURCE_DEPRECATED)
     do_mixed_source_and_assembly_deprecated (gdbarch, uiout, &di, symtab,
-					     low, high, how_many, flags, stb);
+					     low, high, how_many, flags,
+					     stb.get ());
 
-  do_cleanups (cleanups);
   gdb_flush (gdb_stdout);
 }
 
diff --git a/gdb/dummy-frame.c b/gdb/dummy-frame.c
index e81d8e9..f3530b3 100644
--- a/gdb/dummy-frame.c
+++ b/gdb/dummy-frame.c
@@ -409,14 +409,11 @@ maintenance_print_dummy_frames (char *args, int from_tty)
     fprint_dummy_frames (gdb_stdout);
   else
     {
-      struct cleanup *cleanups;
-      struct ui_file *file = gdb_fopen (args, "w");
+      ui_file_up file (gdb_fopen (args, "w"));
 
-      if (file == NULL)
+      if (file.get () == NULL)
 	perror_with_name (_("maintenance print dummy-frames"));
-      cleanups = make_cleanup_ui_file_delete (file);
-      fprint_dummy_frames (file);    
-      do_cleanups (cleanups);
+      fprint_dummy_frames (file.get ());    
     }
 }
 
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index b5dc510..c7e840b 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -8478,21 +8478,20 @@ dwarf2_compute_name (const char *name,
 	{
 	  long length;
 	  const char *prefix;
-	  struct ui_file *buf;
 	  const char *canonical_name = NULL;
 
 	  prefix = determine_prefix (die, cu);
-	  buf = mem_fileopen ();
+	  ui_file_up buf = mem_fileopen ();
 	  if (*prefix != '\0')
 	    {
 	      char *prefixed_name = typename_concat (NULL, prefix, name,
 						     physname, cu);
 
-	      fputs_unfiltered (prefixed_name, buf);
+	      fputs_unfiltered (prefixed_name, buf.get ());
 	      xfree (prefixed_name);
 	    }
 	  else
-	    fputs_unfiltered (name, buf);
+	    fputs_unfiltered (name, buf.get ());
 
 	  /* Template parameters may be specified in the DIE's DW_AT_name, or
 	     as children with DW_TAG_template_type_param or
@@ -8537,25 +8536,26 @@ dwarf2_compute_name (const char *name,
 
 		  if (first)
 		    {
-		      fputs_unfiltered ("<", buf);
+		      fputs_unfiltered ("<", buf.get ());
 		      first = 0;
 		    }
 		  else
-		    fputs_unfiltered (", ", buf);
+		    fputs_unfiltered (", ", buf.get ());
 
 		  attr = dwarf2_attr (child, DW_AT_type, cu);
 		  if (attr == NULL)
 		    {
 		      complaint (&symfile_complaints,
 				 _("template parameter missing DW_AT_type"));
-		      fputs_unfiltered ("UNKNOWN_TYPE", buf);
+		      fputs_unfiltered ("UNKNOWN_TYPE", buf.get ());
 		      continue;
 		    }
 		  type = die_type (child, cu);
 
 		  if (child->tag == DW_TAG_template_type_param)
 		    {
-		      c_print_type (type, "", buf, -1, 0, &type_print_raw_options);
+		      c_print_type (type, "", buf.get (), -1, 0,
+				    &type_print_raw_options);
 		      continue;
 		    }
 
@@ -8565,7 +8565,7 @@ dwarf2_compute_name (const char *name,
 		      complaint (&symfile_complaints,
 				 _("template parameter missing "
 				   "DW_AT_const_value"));
-		      fputs_unfiltered ("UNKNOWN_VALUE", buf);
+		      fputs_unfiltered ("UNKNOWN_VALUE", buf.get ());
 		      continue;
 		    }
 
@@ -8576,7 +8576,7 @@ dwarf2_compute_name (const char *name,
 		  if (TYPE_NOSIGN (type))
 		    /* GDB prints characters as NUMBER 'CHAR'.  If that's
 		       changed, this can use value_print instead.  */
-		    c_printchar (value, type, buf);
+		    c_printchar (value, type, buf.get ());
 		  else
 		    {
 		      struct value_print_options opts;
@@ -8599,7 +8599,7 @@ dwarf2_compute_name (const char *name,
 			 the radix.  */
 		      get_formatted_print_options (&opts, 'd');
 		      opts.raw = 1;
-		      value_print (v, buf, &opts);
+		      value_print (v, buf.get (), &opts);
 		      release_value (v);
 		      value_free (v);
 		    }
@@ -8612,11 +8612,11 @@ dwarf2_compute_name (const char *name,
 		  /* Close the argument list, with a space if necessary
 		     (nested templates).  */
 		  char last_char = '\0';
-		  ui_file_put (buf, do_ui_file_peek_last, &last_char);
+		  ui_file_put (buf.get (), do_ui_file_peek_last, &last_char);
 		  if (last_char == '>')
-		    fputs_unfiltered (" >", buf);
+		    fputs_unfiltered (" >", buf.get ());
 		  else
-		    fputs_unfiltered (">", buf);
+		    fputs_unfiltered (">", buf.get ());
 		}
 	    }
 
@@ -8628,7 +8628,7 @@ dwarf2_compute_name (const char *name,
 	    {
 	      struct type *type = read_type_die (die, cu);
 
-	      c_type_print_args (type, buf, 1, cu->language,
+	      c_type_print_args (type, buf.get (), 1, cu->language,
 				 &type_print_raw_options);
 
 	      if (cu->language == language_cplus)
@@ -8643,12 +8643,11 @@ dwarf2_compute_name (const char *name,
 		      && TYPE_CODE (TYPE_FIELD_TYPE (type, 0)) == TYPE_CODE_PTR
 		      && TYPE_CONST (TYPE_TARGET_TYPE (TYPE_FIELD_TYPE (type,
 									0))))
-		    fputs_unfiltered (" const", buf);
+		    fputs_unfiltered (" const", buf.get ());
 		}
 	    }
 
-	  std::string intermediate_name = ui_file_as_string (buf);
-	  ui_file_delete (buf);
+	  std::string intermediate_name = ui_file_as_string (buf.get ());
 
 	  if (cu->language == language_cplus)
 	    canonical_name
diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
index 8bd0c19..de0484f 100644
--- a/gdb/gdbarch.c
+++ b/gdb/gdbarch.c
@@ -497,17 +497,14 @@ gdbarch_free (struct gdbarch *arch)
 static void
 verify_gdbarch (struct gdbarch *gdbarch)
 {
-  struct ui_file *log;
-  struct cleanup *cleanups;
   long length;
 
-  log = mem_fileopen ();
-  cleanups = make_cleanup_ui_file_delete (log);
+  ui_file_up log = mem_fileopen ();
   /* fundamental */
   if (gdbarch->byte_order == BFD_ENDIAN_UNKNOWN)
-    fprintf_unfiltered (log, "\n\tbyte-order");
+    fprintf_unfiltered (log.get (), "\n\tbyte-order");
   if (gdbarch->bfd_arch_info == NULL)
-    fprintf_unfiltered (log, "\n\tbfd_arch_info");
+    fprintf_unfiltered (log.get (), "\n\tbfd_arch_info");
   /* Check those that need to be defined for the given multi-arch level.  */
   /* Skip verify of bits_big_endian, invalid_p == 0 */
   /* Skip verify of short_bit, invalid_p == 0 */
@@ -542,7 +539,7 @@ verify_gdbarch (struct gdbarch *gdbarch)
   /* Skip verify of pseudo_register_read_value, has predicate.  */
   /* Skip verify of pseudo_register_write, has predicate.  */
   if (gdbarch->num_regs == -1)
-    fprintf_unfiltered (log, "\n\tnum_regs");
+    fprintf_unfiltered (log.get (), "\n\tnum_regs");
   /* Skip verify of num_pseudo_regs, invalid_p == 0 */
   /* Skip verify of ax_pseudo_register_collect, has predicate.  */
   /* Skip verify of ax_pseudo_register_push_stack, has predicate.  */
@@ -556,7 +553,7 @@ verify_gdbarch (struct gdbarch *gdbarch)
   /* Skip verify of sdb_reg_to_regnum, invalid_p == 0 */
   /* Skip verify of dwarf2_reg_to_regnum, invalid_p == 0 */
   if (gdbarch->register_name == 0)
-    fprintf_unfiltered (log, "\n\tregister_name");
+    fprintf_unfiltered (log.get (), "\n\tregister_name");
   /* Skip verify of register_type, has predicate.  */
   /* Skip verify of dummy_id, has predicate.  */
   /* Skip verify of deprecated_fp_regnum, invalid_p == 0 */
@@ -579,14 +576,14 @@ verify_gdbarch (struct gdbarch *gdbarch)
   /* Skip verify of return_value, has predicate.  */
   /* Skip verify of return_in_first_hidden_param_p, invalid_p == 0 */
   if (gdbarch->skip_prologue == 0)
-    fprintf_unfiltered (log, "\n\tskip_prologue");
+    fprintf_unfiltered (log.get (), "\n\tskip_prologue");
   /* Skip verify of skip_main_prologue, has predicate.  */
   /* Skip verify of skip_entrypoint, has predicate.  */
   if (gdbarch->inner_than == 0)
-    fprintf_unfiltered (log, "\n\tinner_than");
+    fprintf_unfiltered (log.get (), "\n\tinner_than");
   /* Skip verify of breakpoint_from_pc, invalid_p == 0 */
   if (gdbarch->breakpoint_kind_from_pc == 0)
-    fprintf_unfiltered (log, "\n\tbreakpoint_kind_from_pc");
+    fprintf_unfiltered (log.get (), "\n\tbreakpoint_kind_from_pc");
   /* Skip verify of sw_breakpoint_from_kind, invalid_p == 0 */
   /* Skip verify of breakpoint_kind_from_current_state, invalid_p == 0 */
   /* Skip verify of adjust_breakpoint_address, has predicate.  */
@@ -607,7 +604,7 @@ verify_gdbarch (struct gdbarch *gdbarch)
   /* Skip verify of software_single_step, has predicate.  */
   /* Skip verify of single_step_through_delay, has predicate.  */
   if (gdbarch->print_insn == 0)
-    fprintf_unfiltered (log, "\n\tprint_insn");
+    fprintf_unfiltered (log.get (), "\n\tprint_insn");
   /* Skip verify of skip_trampoline_code, invalid_p == 0 */
   /* Skip verify of skip_solib_resolver, invalid_p == 0 */
   /* Skip verify of in_solib_return_trampoline, invalid_p == 0 */
@@ -641,9 +638,9 @@ verify_gdbarch (struct gdbarch *gdbarch)
   /* Skip verify of displaced_step_hw_singlestep, invalid_p == 0 */
   /* Skip verify of displaced_step_fixup, has predicate.  */
   if ((! gdbarch->displaced_step_free_closure) != (! gdbarch->displaced_step_copy_insn))
-    fprintf_unfiltered (log, "\n\tdisplaced_step_free_closure");
+    fprintf_unfiltered (log.get (), "\n\tdisplaced_step_free_closure");
   if ((! gdbarch->displaced_step_location) != (! gdbarch->displaced_step_copy_insn))
-    fprintf_unfiltered (log, "\n\tdisplaced_step_location");
+    fprintf_unfiltered (log.get (), "\n\tdisplaced_step_location");
   /* Skip verify of relocate_instruction, has predicate.  */
   /* Skip verify of overlay_update, has predicate.  */
   /* Skip verify of core_read_description, has predicate.  */
@@ -696,12 +693,11 @@ verify_gdbarch (struct gdbarch *gdbarch)
   /* Skip verify of gcc_target_options, invalid_p == 0 */
   /* Skip verify of gnu_triplet_regexp, invalid_p == 0 */
   /* Skip verify of addressable_memory_unit_size, invalid_p == 0 */
-  std::string buf = ui_file_as_string (log);
+  std::string buf = ui_file_as_string (log.get ());
   if (!buf.empty ())
     internal_error (__FILE__, __LINE__,
                     _("verify_gdbarch: the following are invalid ...%s"),
                     buf.c_str ());
-  do_cleanups (cleanups);
 }
 
 
diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh
index 2958cab..a5b0e87 100755
--- a/gdb/gdbarch.sh
+++ b/gdb/gdbarch.sh
@@ -1875,17 +1875,14 @@ cat <<EOF
 static void
 verify_gdbarch (struct gdbarch *gdbarch)
 {
-  struct ui_file *log;
-  struct cleanup *cleanups;
   long length;
 
-  log = mem_fileopen ();
-  cleanups = make_cleanup_ui_file_delete (log);
+  ui_file_up log = mem_fileopen ();
   /* fundamental */
   if (gdbarch->byte_order == BFD_ENDIAN_UNKNOWN)
-    fprintf_unfiltered (log, "\n\tbyte-order");
+    fprintf_unfiltered (log.get (), "\n\tbyte-order");
   if (gdbarch->bfd_arch_info == NULL)
-    fprintf_unfiltered (log, "\n\tbfd_arch_info");
+    fprintf_unfiltered (log.get (), "\n\tbfd_arch_info");
   /* Check those that need to be defined for the given multi-arch level.  */
 EOF
 function_list | while do_read
@@ -1914,21 +1911,20 @@ do
 	elif [ -n "${invalid_p}" ]
 	then
 	    printf "  if (${invalid_p})\n"
-	    printf "    fprintf_unfiltered (log, \"\\\\n\\\\t${function}\");\n"
+	    printf "    fprintf_unfiltered (log.get (), \"\\\\n\\\\t${function}\");\n"
 	elif [ -n "${predefault}" ]
 	then
 	    printf "  if (gdbarch->${function} == ${predefault})\n"
-	    printf "    fprintf_unfiltered (log, \"\\\\n\\\\t${function}\");\n"
+	    printf "    fprintf_unfiltered (log.get (), \"\\\\n\\\\t${function}\");\n"
 	fi
     fi
 done
 cat <<EOF
-  std::string buf = ui_file_as_string (log);
+  std::string buf = ui_file_as_string (log.get ());
   if (!buf.empty ())
     internal_error (__FILE__, __LINE__,
                     _("verify_gdbarch: the following are invalid ...%s"),
                     buf.c_str ());
-  do_cleanups (cleanups);
 }
 EOF
 
diff --git a/gdb/guile/scm-breakpoint.c b/gdb/guile/scm-breakpoint.c
index b2e7c96..f22aee5 100644
--- a/gdb/guile/scm-breakpoint.c
+++ b/gdb/guile/scm-breakpoint.c
@@ -978,8 +978,6 @@ gdbscm_breakpoint_commands (SCM self)
     = bpscm_get_valid_breakpoint_smob_arg_unsafe (self, SCM_ARG1, FUNC_NAME);
   struct breakpoint *bp;
   long length;
-  struct ui_file *string_file;
-  struct cleanup *chain;
   SCM result;
 
   bp = bp_smob->bp;
@@ -987,10 +985,9 @@ gdbscm_breakpoint_commands (SCM self)
   if (bp->commands == NULL)
     return SCM_BOOL_F;
 
-  string_file = mem_fileopen ();
-  chain = make_cleanup_ui_file_delete (string_file);
+  ui_file_up string_file = mem_fileopen ();
 
-  current_uiout->redirect (string_file);
+  current_uiout->redirect (string_file.get ());
   TRY
     {
       print_command_lines (current_uiout, breakpoint_commands (bp), 0);
@@ -998,15 +995,13 @@ gdbscm_breakpoint_commands (SCM self)
   current_uiout->redirect (NULL);
   CATCH (except, RETURN_MASK_ALL)
     {
-      do_cleanups (chain);
       gdbscm_throw_gdb_exception (except);
     }
   END_CATCH
 
-  std::string cmdstr = ui_file_as_string (string_file);
+  std::string cmdstr = ui_file_as_string (string_file.get ());
   result = gdbscm_scm_from_c_string (cmdstr.c_str ());
 
-  do_cleanups (chain);
   return result;
 }
 
diff --git a/gdb/guile/scm-disasm.c b/gdb/guile/scm-disasm.c
index d06c481..ba4f54c 100644
--- a/gdb/guile/scm-disasm.c
+++ b/gdb/guile/scm-disasm.c
@@ -282,33 +282,31 @@ gdbscm_arch_disassemble (SCM self, SCM start_scm, SCM rest)
   for (pc = start, i = 0; pc <= end && i < count; )
     {
       int insn_len = 0;
-      struct ui_file *memfile = mem_fileopen ();
-      struct cleanup *cleanups = make_cleanup_ui_file_delete (memfile);
+      ui_file_up memfile = mem_fileopen ();
 
       TRY
 	{
 	  if (using_port)
 	    {
 	      insn_len = gdbscm_print_insn_from_port (gdbarch, port, offset,
-						      pc, memfile, NULL);
+						      pc, memfile.get (), NULL);
 	    }
 	  else
-	    insn_len = gdb_print_insn (gdbarch, pc, memfile, NULL);
+	    insn_len = gdb_print_insn (gdbarch, pc, memfile.get (), NULL);
 	}
       CATCH (except, RETURN_MASK_ALL)
 	{
-	  GDBSCM_HANDLE_GDB_EXCEPTION_WITH_CLEANUPS (except, cleanups);
+	  gdbscm_throw_gdb_exception (except);
 	}
       END_CATCH
 
-      std::string as = ui_file_as_string (memfile);
+      std::string as = ui_file_as_string (memfile.get ());
 
       result = scm_cons (dascm_make_insn (pc, as.c_str (), insn_len),
 			 result);
 
       pc += insn_len;
       i++;
-      do_cleanups (cleanups);
     }
 
   return scm_reverse_x (result, SCM_EOL);
diff --git a/gdb/guile/scm-frame.c b/gdb/guile/scm-frame.c
index 9b5159e..d73c649 100644
--- a/gdb/guile/scm-frame.c
+++ b/gdb/guile/scm-frame.c
@@ -156,15 +156,14 @@ static int
 frscm_print_frame_smob (SCM self, SCM port, scm_print_state *pstate)
 {
   frame_smob *f_smob = (frame_smob *) SCM_SMOB_DATA (self);
-  struct ui_file *strfile;
 
   gdbscm_printf (port, "#<%s ", frame_smob_name);
 
-  strfile = mem_fileopen ();
-  fprint_frame_id (strfile, f_smob->frame_id);
-  std::string s = ui_file_as_string (strfile);
+  ui_file_up strfile = mem_fileopen ();
+  fprint_frame_id (strfile.get (), f_smob->frame_id);
+  std::string s = ui_file_as_string (strfile.get ());
   gdbscm_printf (port, "%s", s.c_str ());
-  ui_file_delete (strfile);
+  ui_file_delete (strfile.get ());
 
   scm_puts (">", port);
 
diff --git a/gdb/guile/scm-type.c b/gdb/guile/scm-type.c
index f5de011..fbbfbe6 100644
--- a/gdb/guile/scm-type.c
+++ b/gdb/guile/scm-type.c
@@ -107,18 +107,11 @@ tyscm_type_name (struct type *type)
 {
   TRY
     {
-      struct cleanup *old_chain;
-      struct ui_file *stb;
+      ui_file_up stb = mem_fileopen ();
 
-      stb = mem_fileopen ();
-      old_chain = make_cleanup_ui_file_delete (stb);
+      LA_PRINT_TYPE (type, "", stb.get (), -1, 0, &type_print_raw_options);
 
-      LA_PRINT_TYPE (type, "", stb, -1, 0, &type_print_raw_options);
-
-      std::string name = ui_file_as_string (stb);
-      do_cleanups (old_chain);
-
-      return name;
+      return ui_file_as_string (stb.get ());
     }
   CATCH (except, RETURN_MASK_ALL)
     {
diff --git a/gdb/guile/scm-value.c b/gdb/guile/scm-value.c
index b4f32b6..42249df 100644
--- a/gdb/guile/scm-value.c
+++ b/gdb/guile/scm-value.c
@@ -157,15 +157,12 @@ vlscm_print_value_smob (SCM self, SCM port, scm_print_state *pstate)
 
   TRY
     {
-      struct ui_file *stb = mem_fileopen ();
-      struct cleanup *old_chain = make_cleanup_ui_file_delete (stb);
+      ui_file_up stb = mem_fileopen ();
 
-      common_val_print (v_smob->value, stb, 0, &opts, current_language);
+      common_val_print (v_smob->value, stb.get (), 0, &opts, current_language);
 
-      std::string s = ui_file_as_string (stb);
+      std::string s = ui_file_as_string (stb.get ());
       scm_puts (s.c_str (), port);
-
-      do_cleanups (old_chain);
     }
   CATCH (except, RETURN_MASK_ALL)
     {
@@ -1285,13 +1282,10 @@ gdbscm_value_print (SCM self)
 
   TRY
     {
-      struct ui_file *stb = mem_fileopen ();
-      struct cleanup *old_chain = make_cleanup_ui_file_delete (stb);
-
-      common_val_print (value, stb, 0, &opts, current_language);
-      s = ui_file_as_string (stb);
+      ui_file_up stb = mem_fileopen ();
 
-      do_cleanups (old_chain);
+      common_val_print (value, stb.get (), 0, &opts, current_language);
+      s = ui_file_as_string (stb.get ());
     }
   CATCH (except, RETURN_MASK_ALL)
     {
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index d761117..4182ae3 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -1670,21 +1670,17 @@ print_return_value_1 (struct ui_out *uiout, struct return_value_info *rv)
   if (rv->value != NULL)
     {
       struct value_print_options opts;
-      struct ui_file *stb;
-      struct cleanup *old_chain;
+      ui_file_up stb = mem_fileopen ();
 
       /* Print it.  */
-      stb = mem_fileopen ();
-      old_chain = make_cleanup_ui_file_delete (stb);
       uiout->text ("Value returned is ");
       uiout->field_fmt ("gdb-result-var", "$%d",
 			rv->value_history_index);
       uiout->text (" = ");
       get_no_prettyformat_print_options (&opts);
-      value_print (rv->value, stb, &opts);
-      uiout->field_stream ("return-value", stb);
+      value_print (rv->value, stb.get (), &opts);
+      uiout->field_stream ("return-value", stb.get ());
       uiout->text ("\n");
-      do_cleanups (old_chain);
     }
   else
     {
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 41f1fdd..fc79ad5 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -3438,40 +3438,39 @@ print_target_wait_results (ptid_t waiton_ptid, ptid_t result_ptid,
 			   const struct target_waitstatus *ws)
 {
   char *status_string = target_waitstatus_to_string (ws);
-  struct ui_file *tmp_stream = mem_fileopen ();
+  ui_file_up tmp_stream = mem_fileopen ();
 
   /* The text is split over several lines because it was getting too long.
      Call fprintf_unfiltered (gdb_stdlog) once so that the text is still
      output as a unit; we want only one timestamp printed if debug_timestamp
      is set.  */
 
-  fprintf_unfiltered (tmp_stream,
+  fprintf_unfiltered (tmp_stream.get (),
 		      "infrun: target_wait (%d.%ld.%ld",
 		      ptid_get_pid (waiton_ptid),
 		      ptid_get_lwp (waiton_ptid),
 		      ptid_get_tid (waiton_ptid));
   if (ptid_get_pid (waiton_ptid) != -1)
-    fprintf_unfiltered (tmp_stream,
+    fprintf_unfiltered (tmp_stream.get (),
 			" [%s]", target_pid_to_str (waiton_ptid));
-  fprintf_unfiltered (tmp_stream, ", status) =\n");
-  fprintf_unfiltered (tmp_stream,
+  fprintf_unfiltered (tmp_stream.get (), ", status) =\n");
+  fprintf_unfiltered (tmp_stream.get (),
 		      "infrun:   %d.%ld.%ld [%s],\n",
 		      ptid_get_pid (result_ptid),
 		      ptid_get_lwp (result_ptid),
 		      ptid_get_tid (result_ptid),
 		      target_pid_to_str (result_ptid));
-  fprintf_unfiltered (tmp_stream,
+  fprintf_unfiltered (tmp_stream.get (),
 		      "infrun:   %s\n",
 		      status_string);
 
-  std::string text = ui_file_as_string (tmp_stream);
+  std::string text = ui_file_as_string (tmp_stream.get ());
 
   /* This uses %s in part to handle %'s in the text, but also to avoid
      a gcc error: the format attribute requires a string literal.  */
   fprintf_unfiltered (gdb_stdlog, "%s", text.c_str ());
 
   xfree (status_string);
-  ui_file_delete (tmp_stream);
 }
 
 /* Select a thread at random, out of those which are resumed and have
diff --git a/gdb/language.c b/gdb/language.c
index a40eb87..4ea7f79 100644
--- a/gdb/language.c
+++ b/gdb/language.c
@@ -540,7 +540,6 @@ add_language (const struct language_defn *lang)
   /* For the "help set language" command.  */
 
   int i;
-  struct ui_file *tmp_stream;
 
   if (lang->la_magic != LANG_MAGIC)
     {
@@ -584,9 +583,9 @@ add_language (const struct language_defn *lang)
     }
 
   /* Build the "help set language" docs.  */
-  tmp_stream = mem_fileopen ();
+  ui_file_up tmp_stream = mem_fileopen ();
 
-  fprintf_unfiltered (tmp_stream,
+  fprintf_unfiltered (tmp_stream.get (),
 		      _("Set the current source language.\n"
 			"The currently understood settings are:\n\nlocal or "
 			"auto    Automatic setting based on source file\n"));
@@ -600,7 +599,7 @@ add_language (const struct language_defn *lang)
 
       /* FIXME: i18n: for now assume that the human-readable name
 	 is just a capitalization of the internal name.  */
-      fprintf_unfiltered (tmp_stream, "%-16s Use the %c%s language\n",
+      fprintf_unfiltered (tmp_stream.get (), "%-16s Use the %c%s language\n",
 			  languages[i]->la_name,
 			  /* Capitalize first letter of language
 			     name.  */
@@ -608,8 +607,7 @@ add_language (const struct language_defn *lang)
 			  languages[i]->la_name + 1);
     }
 
-  std::string language_set_doc = ui_file_as_string (tmp_stream);
-  ui_file_delete (tmp_stream);
+  std::string language_set_doc = ui_file_as_string (tmp_stream.get ());
 
   add_setshow_enum_cmd ("language", class_support,
 			(const char **) language_names,
diff --git a/gdb/location.c b/gdb/location.c
index 37da6df..8e54248 100644
--- a/gdb/location.c
+++ b/gdb/location.c
@@ -227,58 +227,54 @@ static char *
 explicit_to_string_internal (int as_linespec,
 			     const struct explicit_location *explicit_loc)
 {
-  struct ui_file *buf;
   char space, *result;
   int need_space = 0;
-  struct cleanup *cleanup;
 
   space = as_linespec ? ':' : ' ';
-  buf = mem_fileopen ();
-  cleanup = make_cleanup_ui_file_delete (buf);
+  ui_file_up buf = mem_fileopen ();
 
   if (explicit_loc->source_filename != NULL)
     {
       if (!as_linespec)
-	fputs_unfiltered ("-source ", buf);
-      fputs_unfiltered (explicit_loc->source_filename, buf);
+	fputs_unfiltered ("-source ", buf.get ());
+      fputs_unfiltered (explicit_loc->source_filename, buf.get ());
       need_space = 1;
     }
 
   if (explicit_loc->function_name != NULL)
     {
       if (need_space)
-	fputc_unfiltered (space, buf);
+	fputc_unfiltered (space, buf.get ());
       if (!as_linespec)
-	fputs_unfiltered ("-function ", buf);
-      fputs_unfiltered (explicit_loc->function_name, buf);
+	fputs_unfiltered ("-function ", buf.get ());
+      fputs_unfiltered (explicit_loc->function_name, buf.get ());
       need_space = 1;
     }
 
   if (explicit_loc->label_name != NULL)
     {
       if (need_space)
-	fputc_unfiltered (space, buf);
+	fputc_unfiltered (space, buf.get ());
       if (!as_linespec)
-	fputs_unfiltered ("-label ", buf);
-      fputs_unfiltered (explicit_loc->label_name, buf);
+	fputs_unfiltered ("-label ", buf.get ());
+      fputs_unfiltered (explicit_loc->label_name, buf.get ());
       need_space = 1;
     }
 
   if (explicit_loc->line_offset.sign != LINE_OFFSET_UNKNOWN)
     {
       if (need_space)
-	fputc_unfiltered (space, buf);
+	fputc_unfiltered (space, buf.get ());
       if (!as_linespec)
-	fputs_unfiltered ("-line ", buf);
-      fprintf_filtered (buf, "%s%d",
+	fputs_unfiltered ("-line ", buf.get ());
+      fprintf_filtered (buf.get (), "%s%d",
 			(explicit_loc->line_offset.sign == LINE_OFFSET_NONE ? ""
 			 : (explicit_loc->line_offset.sign
 			    == LINE_OFFSET_PLUS ? "+" : "-")),
 			explicit_loc->line_offset.offset);
     }
 
-  result = ui_file_xstrdup (buf, NULL);
-  do_cleanups (cleanup);
+  result = ui_file_xstrdup (buf.get (), NULL);
   return result;
 }
 
diff --git a/gdb/maint.c b/gdb/maint.c
index 77b56af..7235256 100644
--- a/gdb/maint.c
+++ b/gdb/maint.c
@@ -403,14 +403,11 @@ maintenance_print_architecture (char *args, int from_tty)
     gdbarch_dump (gdbarch, gdb_stdout);
   else
     {
-      struct cleanup *cleanups;
-      struct ui_file *file = gdb_fopen (args, "w");
+      ui_file_up file (gdb_fopen (args, "w"));
 
       if (file == NULL)
 	perror_with_name (_("maintenance print architecture"));
-      cleanups = make_cleanup_ui_file_delete (file);
-      gdbarch_dump (gdbarch, file);
-      do_cleanups (cleanups);
+      gdbarch_dump (gdbarch, file.get ());
     }
 }
 
diff --git a/gdb/mi/mi-cmd-stack.c b/gdb/mi/mi-cmd-stack.c
index efaf49d..acef556 100644
--- a/gdb/mi/mi-cmd-stack.c
+++ b/gdb/mi/mi-cmd-stack.c
@@ -488,7 +488,6 @@ list_arg_or_local (const struct frame_arg *arg, enum what_to_list what,
 {
   struct cleanup *old_chain;
   struct ui_out *uiout = current_uiout;
-  struct ui_file *stb;
 
   gdb_assert (!arg->val || !arg->error);
   gdb_assert ((values == PRINT_NO_VALUES && arg->val == NULL
@@ -511,16 +510,16 @@ list_arg_or_local (const struct frame_arg *arg, enum what_to_list what,
 					 TYPE_LENGTH (value_type (arg->val))))))
     return;
 
-  stb = mem_fileopen ();
-  old_chain = make_cleanup_ui_file_delete (stb);
+  ui_file_up stb = mem_fileopen ();
+  old_chain = make_cleanup (null_cleanup, NULL);
 
   if (values != PRINT_NO_VALUES || what == all)
     make_cleanup_ui_out_tuple_begin_end (uiout, NULL);
 
-  fputs_filtered (SYMBOL_PRINT_NAME (arg->sym), stb);
+  fputs_filtered (SYMBOL_PRINT_NAME (arg->sym), stb.get ());
   if (arg->entry_kind == print_entry_values_only)
-    fputs_filtered ("@entry", stb);
-  uiout->field_stream ("name", stb);
+    fputs_filtered ("@entry", stb.get ());
+  uiout->field_stream ("name", stb.get ());
 
   if (what == all && SYMBOL_IS_ARGUMENT (arg->sym))
     uiout->field_int ("arg", 1);
@@ -528,8 +527,8 @@ list_arg_or_local (const struct frame_arg *arg, enum what_to_list what,
   if (values == PRINT_SIMPLE_VALUES)
     {
       check_typedef (arg->sym->type);
-      type_print (arg->sym->type, "", stb, -1);
-      uiout->field_stream ("type", stb);
+      type_print (arg->sym->type, "", stb.get (), -1);
+      uiout->field_stream ("type", stb.get ());
     }
 
   if (arg->val || arg->error)
@@ -546,7 +545,7 @@ list_arg_or_local (const struct frame_arg *arg, enum what_to_list what,
 
 	      get_no_prettyformat_print_options (&opts);
 	      opts.deref_ref = 1;
-	      common_val_print (arg->val, stb, 0, &opts,
+	      common_val_print (arg->val, stb.get (), 0, &opts,
 				language_def (SYMBOL_LANGUAGE (arg->sym)));
 	    }
 	  CATCH (except, RETURN_MASK_ERROR)
@@ -556,9 +555,9 @@ list_arg_or_local (const struct frame_arg *arg, enum what_to_list what,
 	  END_CATCH
 	}
       if (error_message != NULL)
-	fprintf_filtered (stb, _("<error reading variable: %s>"),
+	fprintf_filtered (stb.get (), _("<error reading variable: %s>"),
 			  error_message);
-      uiout->field_stream ("value", stb);
+      uiout->field_stream ("value", stb.get ());
     }
 
   do_cleanups (old_chain);
diff --git a/gdb/mi/mi-console.c b/gdb/mi/mi-console.c
index afb5e94..0c7727f 100644
--- a/gdb/mi/mi-console.c
+++ b/gdb/mi/mi-console.c
@@ -54,7 +54,7 @@ mi_console_file_new (struct ui_file *raw, const char *prefix, char quote)
 
   mi_console->magic = &mi_console_file_magic;
   mi_console->raw = raw;
-  mi_console->buffer = mem_fileopen ();
+  mi_console->buffer = mem_fileopen ().release ();
   mi_console->prefix = prefix;
   mi_console->quote = quote;
   set_ui_file_fputs (ui_file, mi_console_file_fputs);
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index 22803cb..0411a95 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -1261,7 +1261,6 @@ output_register (struct frame_info *frame, int regnum, int format,
   struct value *val = value_of_register (regnum, frame);
   struct cleanup *tuple_cleanup;
   struct value_print_options opts;
-  struct ui_file *stb;
 
   if (skip_unavailable && !value_entirely_available (val))
     return;
@@ -1275,15 +1274,14 @@ output_register (struct frame_info *frame, int regnum, int format,
   if (format == 'r')
     format = 'z';
 
-  stb = mem_fileopen ();
-  make_cleanup_ui_file_delete (stb);
+  ui_file_up stb = mem_fileopen ();
 
   get_formatted_print_options (&opts, format);
   opts.deref_ref = 1;
   val_print (value_type (val),
 	     value_embedded_offset (val), 0,
-	     stb, 0, val, &opts, current_language);
-  uiout->field_stream ("value", stb);
+	     stb.get (), 0, val, &opts, current_language);
+  uiout->field_stream ("value", stb.get ());
 
   do_cleanups (tuple_cleanup);
 }
@@ -1352,14 +1350,11 @@ mi_cmd_data_write_register_values (char *command, char **argv, int argc)
 void
 mi_cmd_data_evaluate_expression (char *command, char **argv, int argc)
 {
-  struct cleanup *old_chain;
   struct value *val;
-  struct ui_file *stb;
   struct value_print_options opts;
   struct ui_out *uiout = current_uiout;
 
-  stb = mem_fileopen ();
-  old_chain = make_cleanup_ui_file_delete (stb);
+  ui_file_up stb = mem_fileopen ();
 
   if (argc != 1)
     error (_("-data-evaluate-expression: "
@@ -1372,11 +1367,9 @@ mi_cmd_data_evaluate_expression (char *command, char **argv, int argc)
   /* Print the result of the expression evaluation.  */
   get_user_print_options (&opts);
   opts.deref_ref = 0;
-  common_val_print (val, stb, 0, &opts, current_language);
-
-  uiout->field_stream ("value", stb);
+  common_val_print (val, stb.get (), 0, &opts, current_language);
 
-  do_cleanups (old_chain);
+  uiout->field_stream ("value", stb.get ());
 }
 
 /* This is the -data-read-memory command.
@@ -1516,15 +1509,13 @@ mi_cmd_data_read_memory (char *command, char **argv, int argc)
 
   /* Build the result as a two dimentional table.  */
   {
-    struct ui_file *stream;
-    struct cleanup *cleanup_stream;
+    struct cleanup *cleanup_list;
     int row;
     int row_byte;
 
-    stream = mem_fileopen ();
-    cleanup_stream = make_cleanup_ui_file_delete (stream);
+    ui_file_up stream = mem_fileopen ();
 
-    make_cleanup_ui_out_list_begin_end (uiout, "memory");
+    cleanup_list = make_cleanup_ui_out_list_begin_end (uiout, "memory");
     for (row = 0, row_byte = 0;
 	 row < nr_rows;
 	 row++, row_byte += nr_cols * word_size)
@@ -1551,10 +1542,10 @@ mi_cmd_data_read_memory (char *command, char **argv, int argc)
 	      }
 	    else
 	      {
-		ui_file_rewind (stream);
+		ui_file_rewind (stream.get ());
 		print_scalar_formatted (&mbuf[col_byte], word_type, &opts,
-					word_asize, stream);
-		uiout->field_stream (NULL, stream);
+					word_asize, stream.get ());
+		uiout->field_stream (NULL, stream.get ());
 	      }
 	  }
 	do_cleanups (cleanup_list_data);
@@ -1562,22 +1553,22 @@ mi_cmd_data_read_memory (char *command, char **argv, int argc)
 	  {
 	    int byte;
 
-	    ui_file_rewind (stream);
+	    ui_file_rewind (stream.get ());
 	    for (byte = row_byte;
 		 byte < row_byte + word_size * nr_cols; byte++)
 	      {
 		if (byte >= nr_bytes)
-		  fputc_unfiltered ('X', stream);
+		  fputc_unfiltered ('X', stream.get ());
 		else if (mbuf[byte] < 32 || mbuf[byte] > 126)
-		  fputc_unfiltered (aschar, stream);
+		  fputc_unfiltered (aschar, stream.get ());
 		else
-		  fputc_unfiltered (mbuf[byte], stream);
+		  fputc_unfiltered (mbuf[byte], stream.get ());
 	      }
-	    uiout->field_stream ("ascii", stream);
+	    uiout->field_stream ("ascii", stream.get ());
 	  }
 	do_cleanups (cleanup_tuple);
       }
-    do_cleanups (cleanup_stream);
+    do_cleanups (cleanup_list);
   }
 }
 
@@ -2311,16 +2302,13 @@ mi_cmd_execute (struct mi_parse *parse)
   else
     {
       /* FIXME: DELETE THIS.  */
-      struct ui_file *stb;
-
-      stb = mem_fileopen ();
+      ui_file_up stb = mem_fileopen ();
 
-      fputs_unfiltered ("Undefined mi command: ", stb);
-      fputstr_unfiltered (parse->command, '"', stb);
-      fputs_unfiltered (" (missing implementation)", stb);
+      fputs_unfiltered ("Undefined mi command: ", stb.get ());
+      fputstr_unfiltered (parse->command, '"', stb.get ());
+      fputs_unfiltered (" (missing implementation)", stb.get ());
 
-      make_cleanup_ui_file_delete (stb);
-      error_stream (stb);
+      error_stream (stb.get ());
     }
   do_cleanups (cleanup);
 }
@@ -2699,12 +2687,11 @@ print_variable_or_computed (const char *expression, enum print_values values)
 {
   struct cleanup *old_chain;
   struct value *val;
-  struct ui_file *stb;
   struct type *type;
   struct ui_out *uiout = current_uiout;
 
-  stb = mem_fileopen ();
-  old_chain = make_cleanup_ui_file_delete (stb);
+  ui_file_up stb = mem_fileopen ();
+  old_chain = make_cleanup (null_cleanup, NULL);
 
   expression_up expr = parse_expression (expression);
 
@@ -2721,8 +2708,8 @@ print_variable_or_computed (const char *expression, enum print_values values)
     {
     case PRINT_SIMPLE_VALUES:
       type = check_typedef (value_type (val));
-      type_print (value_type (val), "", stb, -1);
-      uiout->field_stream ("type", stb);
+      type_print (value_type (val), "", stb.get (), -1);
+      uiout->field_stream ("type", stb.get ());
       if (TYPE_CODE (type) != TYPE_CODE_ARRAY
 	  && TYPE_CODE (type) != TYPE_CODE_STRUCT
 	  && TYPE_CODE (type) != TYPE_CODE_UNION)
@@ -2731,8 +2718,8 @@ print_variable_or_computed (const char *expression, enum print_values values)
 
 	  get_no_prettyformat_print_options (&opts);
 	  opts.deref_ref = 1;
-	  common_val_print (val, stb, 0, &opts, current_language);
-	  uiout->field_stream ("value", stb);
+	  common_val_print (val, stb.get (), 0, &opts, current_language);
+	  uiout->field_stream ("value", stb.get ());
 	}
       break;
     case PRINT_ALL_VALUES:
@@ -2741,8 +2728,8 @@ print_variable_or_computed (const char *expression, enum print_values values)
 
 	get_no_prettyformat_print_options (&opts);
 	opts.deref_ref = 1;
-	common_val_print (val, stb, 0, &opts, current_language);
-	uiout->field_stream ("value", stb);
+	common_val_print (val, stb.get (), 0, &opts, current_language);
+	uiout->field_stream ("value", stb.get ());
       }
       break;
     }
diff --git a/gdb/mi/mi-out.c b/gdb/mi/mi-out.c
index 5a5aef9..cba23c9 100644
--- a/gdb/mi/mi-out.c
+++ b/gdb/mi/mi-out.c
@@ -284,7 +284,7 @@ mi_ui_out::~mi_ui_out ()
 mi_ui_out *
 mi_out_new (int mi_version)
 {
-  ui_file *stream = mem_fileopen ();
+  ui_file *stream = mem_fileopen ().release ();
 
   return new mi_ui_out (mi_version, stream);
 }
diff --git a/gdb/printcmd.c b/gdb/printcmd.c
index e4711e9..4f04fe3 100644
--- a/gdb/printcmd.c
+++ b/gdb/printcmd.c
@@ -2717,18 +2717,15 @@ printf_command (char *arg, int from_tty)
 static void
 eval_command (char *arg, int from_tty)
 {
-  struct ui_file *ui_out = mem_fileopen ();
-  struct cleanup *cleanups = make_cleanup_ui_file_delete (ui_out);
+  ui_file_up ui_out = mem_fileopen ();
 
-  ui_printf (arg, ui_out);
+  ui_printf (arg, ui_out.get ());
 
-  std::string expanded = ui_file_as_string (ui_out);
+  std::string expanded = ui_file_as_string (ui_out.get ());
 
   expanded = insert_user_defined_cmd_args (expanded.c_str ());
 
   execute_command (&expanded[0], from_tty);
-
-  do_cleanups (cleanups);
 }
 
 void
diff --git a/gdb/python/py-arch.c b/gdb/python/py-arch.c
index 13cc9ba..65d5ee4 100644
--- a/gdb/python/py-arch.c
+++ b/gdb/python/py-arch.c
@@ -193,36 +193,26 @@ archpy_disassemble (PyObject *self, PyObject *args, PyObject *kw)
        || (end_obj == NULL && count_obj == NULL && pc == start);)
     {
       int insn_len = 0;
-      struct ui_file *memfile = mem_fileopen ();
+      ui_file_up memfile = mem_fileopen ();
       gdbpy_ref insn_dict (PyDict_New ());
 
       if (insn_dict == NULL)
-        {
-          ui_file_delete (memfile);
-
-          return NULL;
-        }
+	return NULL;
       if (PyList_Append (result_list.get (), insn_dict.get ()))
-        {
-          ui_file_delete (memfile);
-
-          return NULL;  /* PyList_Append Sets the exception.  */
-        }
+	return NULL;  /* PyList_Append Sets the exception.  */
 
       TRY
         {
-          insn_len = gdb_print_insn (gdbarch, pc, memfile, NULL);
+          insn_len = gdb_print_insn (gdbarch, pc, memfile.get (), NULL);
         }
       CATCH (except, RETURN_MASK_ALL)
         {
-          ui_file_delete (memfile);
-
 	  gdbpy_convert_exception (except);
 	  return NULL;
         }
       END_CATCH
 
-      std::string as = ui_file_as_string (memfile);
+      std::string as = ui_file_as_string (memfile.get ());
 
       if (PyDict_SetItemString (insn_dict.get (), "addr",
                                 gdb_py_long_from_ulongest (pc))
@@ -232,15 +222,10 @@ archpy_disassemble (PyObject *self, PyObject *args, PyObject *kw)
 							: "<unknown>"))
           || PyDict_SetItemString (insn_dict.get (), "length",
                                    PyInt_FromLong (insn_len)))
-        {
-          ui_file_delete (memfile);
-
-          return NULL;
-        }
+	return NULL;
 
       pc += insn_len;
       i++;
-      ui_file_delete (memfile);
     }
 
   return result_list.release ();
diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c
index f268b2b..f086725 100644
--- a/gdb/python/py-breakpoint.c
+++ b/gdb/python/py-breakpoint.c
@@ -485,19 +485,16 @@ bppy_get_commands (PyObject *self, void *closure)
   gdbpy_breakpoint_object *self_bp = (gdbpy_breakpoint_object *) self;
   struct breakpoint *bp = self_bp->bp;
   long length;
-  struct ui_file *string_file;
   PyObject *result;
-  struct cleanup *chain;
 
   BPPY_REQUIRE_VALID (self_bp);
 
   if (! self_bp->bp->commands)
     Py_RETURN_NONE;
 
-  string_file = mem_fileopen ();
-  chain = make_cleanup_ui_file_delete (string_file);
+  ui_file_up string_file = mem_fileopen ();
 
-  current_uiout->redirect (string_file);
+  current_uiout->redirect (string_file.get ());
   TRY
     {
       print_command_lines (current_uiout, breakpoint_commands (bp), 0);
@@ -505,16 +502,14 @@ bppy_get_commands (PyObject *self, void *closure)
   CATCH (except, RETURN_MASK_ALL)
     {
       current_uiout->redirect (NULL);
-      do_cleanups (chain);
       gdbpy_convert_exception (except);
       return NULL;
     }
   END_CATCH
 
   current_uiout->redirect (NULL);
-  std::string cmdstr = ui_file_as_string (string_file);
+  std::string cmdstr = ui_file_as_string (string_file.get ());
   result = host_string_to_python_string (cmdstr.c_str ());
-  do_cleanups (chain);
   return result;
 }
 
diff --git a/gdb/python/py-frame.c b/gdb/python/py-frame.c
index 048e7c1..3594be2 100644
--- a/gdb/python/py-frame.c
+++ b/gdb/python/py-frame.c
@@ -81,11 +81,10 @@ static PyObject *
 frapy_str (PyObject *self)
 {
   PyObject *result;
-  struct ui_file *strfile;
 
-  strfile = mem_fileopen ();
-  fprint_frame_id (strfile, ((frame_object *) self)->frame_id);
-  std::string s = ui_file_as_string (strfile);
+  ui_file_up strfile = mem_fileopen ();
+  fprint_frame_id (strfile.get (), ((frame_object *) self)->frame_id);
+  std::string s = ui_file_as_string (strfile.get ());
   return PyString_FromString (s.c_str ());
 }
 
diff --git a/gdb/python/py-framefilter.c b/gdb/python/py-framefilter.c
index ae5197a..dd5b5d4 100644
--- a/gdb/python/py-framefilter.c
+++ b/gdb/python/py-framefilter.c
@@ -209,15 +209,10 @@ py_print_type (struct ui_out *out, struct value *val)
 
   TRY
     {
-      struct ui_file *stb;
-      struct cleanup *cleanup;
-
-      stb = mem_fileopen ();
-      cleanup = make_cleanup_ui_file_delete (stb);
+      ui_file_up stb = mem_fileopen ();
       check_typedef (value_type (val));
-      type_print (value_type (val), "", stb, -1);
-      out->field_stream ("type", stb);
-      do_cleanups (cleanup);
+      type_print (value_type (val), "", stb.get (), -1);
+      out->field_stream ("type", stb.get ());
     }
   CATCH (except, RETURN_MASK_ALL)
     {
@@ -281,14 +276,9 @@ py_print_value (struct ui_out *out, struct value *val,
     {
       TRY
 	{
-	  struct ui_file *stb;
-	  struct cleanup *cleanup;
-
-	  stb = mem_fileopen ();
-	  cleanup = make_cleanup_ui_file_delete (stb);
-	  common_val_print (val, stb, indent, opts, language);
-	  out->field_stream ("value", stb);
-	  do_cleanups (cleanup);
+	  ui_file_up stb = mem_fileopen ();
+	  common_val_print (val, stb.get (), indent, opts, language);
+	  out->field_stream ("value", stb.get ());
 	}
       CATCH (except, RETURN_MASK_ALL)
 	{
@@ -394,29 +384,24 @@ py_print_single_arg (struct ui_out *out,
 	 entry value options.  */
       if (fa != NULL)
 	{
-	  struct ui_file *stb;
-	  struct cleanup *cleanup;
-
-	  stb = mem_fileopen ();
-	  cleanup = make_cleanup_ui_file_delete (stb);
-	  fprintf_symbol_filtered (stb, SYMBOL_PRINT_NAME (fa->sym),
+	  ui_file_up stb = mem_fileopen ();
+	  fprintf_symbol_filtered (stb.get (), SYMBOL_PRINT_NAME (fa->sym),
 				   SYMBOL_LANGUAGE (fa->sym),
 				   DMGL_PARAMS | DMGL_ANSI);
 	  if (fa->entry_kind == print_entry_values_compact)
 	    {
-	      fputs_filtered ("=", stb);
+	      fputs_filtered ("=", stb.get ());
 
-	      fprintf_symbol_filtered (stb, SYMBOL_PRINT_NAME (fa->sym),
+	      fprintf_symbol_filtered (stb.get (), SYMBOL_PRINT_NAME (fa->sym),
 				       SYMBOL_LANGUAGE (fa->sym),
 				       DMGL_PARAMS | DMGL_ANSI);
 	    }
 	  if (fa->entry_kind == print_entry_values_only
 	      || fa->entry_kind == print_entry_values_compact)
 	    {
-	      fputs_filtered ("@entry", stb);
+	      fputs_filtered ("@entry", stb.get ());
 	    }
-	  out->field_stream ("name", stb);
-	  do_cleanups (cleanup);
+	  out->field_stream ("name", stb.get ());
 	}
       else
 	/* Otherwise, just output the name.  */
diff --git a/gdb/python/py-type.c b/gdb/python/py-type.c
index 7862829..235e8b5 100644
--- a/gdb/python/py-type.c
+++ b/gdb/python/py-type.c
@@ -972,17 +972,12 @@ typy_str (PyObject *self)
 
   TRY
     {
-      struct cleanup *old_chain;
-      struct ui_file *stb;
+      ui_file_up stb = mem_fileopen ();
 
-      stb = mem_fileopen ();
-      old_chain = make_cleanup_ui_file_delete (stb);
-
-      LA_PRINT_TYPE (type_object_to_type (self), "", stb, -1, 0,
+      LA_PRINT_TYPE (type_object_to_type (self), "", stb.get (), -1, 0,
 		     &type_print_raw_options);
 
-      thetype = ui_file_as_string (stb);
-      do_cleanups (old_chain);
+      thetype = ui_file_as_string (stb.get ());
     }
   CATCH (except, RETURN_MASK_ALL)
     {
diff --git a/gdb/python/py-unwind.c b/gdb/python/py-unwind.c
index 690412a..9485583 100644
--- a/gdb/python/py-unwind.c
+++ b/gdb/python/py-unwind.c
@@ -198,12 +198,12 @@ pyuw_object_attribute_to_pointer (PyObject *pyo, const char *attr_name,
 static PyObject *
 unwind_infopy_str (PyObject *self)
 {
-  struct ui_file *strfile = mem_fileopen ();
+  ui_file_up strfile = mem_fileopen ();
   unwind_info_object *unwind_info = (unwind_info_object *) self;
   PyObject *result;
 
-  fprintf_unfiltered (strfile, "Frame ID: ");
-  fprint_frame_id (strfile, unwind_info->frame_id);
+  fprintf_unfiltered (strfile.get (), "Frame ID: ");
+  fprint_frame_id (strfile.get (), unwind_info->frame_id);
   {
     char *sep = "";
     int i;
@@ -211,18 +211,18 @@ unwind_infopy_str (PyObject *self)
     saved_reg *reg;
 
     get_user_print_options (&opts);
-    fprintf_unfiltered (strfile, "\nSaved registers: (");
+    fprintf_unfiltered (strfile.get (), "\nSaved registers: (");
     for (i = 0; VEC_iterate (saved_reg, unwind_info->saved_regs, i, reg); i++)
       {
         struct value *value = value_object_to_value (reg->value);
 
-        fprintf_unfiltered (strfile, "%s(%d, ", sep, reg->number);
+        fprintf_unfiltered (strfile.get (), "%s(%d, ", sep, reg->number);
         if (value != NULL)
           {
             TRY
               {
-                value_print (value, strfile, &opts);
-                fprintf_unfiltered (strfile, ")");
+                value_print (value, strfile.get (), &opts);
+                fprintf_unfiltered (strfile.get (), ")");
               }
             CATCH (except, RETURN_MASK_ALL)
               {
@@ -231,15 +231,14 @@ unwind_infopy_str (PyObject *self)
             END_CATCH
           }
         else
-          fprintf_unfiltered (strfile, "<BAD>)");
+          fprintf_unfiltered (strfile.get (), "<BAD>)");
         sep = ", ";
       }
-    fprintf_unfiltered (strfile, ")");
+    fprintf_unfiltered (strfile.get (), ")");
   }
 
-  std::string s = ui_file_as_string (strfile);
+  std::string s = ui_file_as_string (strfile.get ());
   result = PyString_FromString (s.c_str ());
-  ui_file_delete (strfile);
   return result;
 }
 
diff --git a/gdb/python/py-value.c b/gdb/python/py-value.c
index 8f3164b..17935e7 100644
--- a/gdb/python/py-value.c
+++ b/gdb/python/py-value.c
@@ -876,14 +876,11 @@ valpy_str (PyObject *self)
 
   TRY
     {
-      struct ui_file *stb = mem_fileopen ();
-      struct cleanup *old_chain = make_cleanup_ui_file_delete (stb);
+      ui_file_up stb = mem_fileopen ();
 
-      common_val_print (((value_object *) self)->value, stb, 0,
+      common_val_print (((value_object *) self)->value, stb.get (), 0,
 			&opts, python_language);
-      s = ui_file_as_string (stb);
-
-      do_cleanups (old_chain);
+      s = ui_file_as_string (stb.get ());
     }
   CATCH (except, RETURN_MASK_ALL)
     {
diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c
index 6cba1d2..e3b5419 100644
--- a/gdb/record-btrace.c
+++ b/gdb/record-btrace.c
@@ -696,7 +696,6 @@ btrace_insn_history (struct ui_out *uiout,
 		     const struct btrace_insn_iterator *begin,
 		     const struct btrace_insn_iterator *end, int flags)
 {
-  struct ui_file *stb;
   struct cleanup *cleanups, *ui_item_chain;
   struct disassemble_info di;
   struct gdbarch *gdbarch;
@@ -709,12 +708,11 @@ btrace_insn_history (struct ui_out *uiout,
   flags |= DISASSEMBLY_SPECULATIVE;
 
   gdbarch = target_gdbarch ();
-  stb = mem_fileopen ();
-  cleanups = make_cleanup_ui_file_delete (stb);
-  di = gdb_disassemble_info (gdbarch, stb);
+  ui_file_up stb = mem_fileopen ();
+  di = gdb_disassemble_info (gdbarch, stb.get ());
   last_lines = btrace_mk_line_range (NULL, 0, 0);
 
-  make_cleanup_ui_out_list_begin_end (uiout, "asm_insns");
+  cleanups = make_cleanup_ui_out_list_begin_end (uiout, "asm_insns");
 
   /* UI_ITEM_CHAIN is a cleanup chain for the last source line and the
      instructions corresponding to that line.  */
@@ -773,7 +771,8 @@ btrace_insn_history (struct ui_out *uiout,
 	  if ((insn->flags & BTRACE_INSN_FLAG_SPECULATIVE) != 0)
 	    dinsn.is_speculative = 1;
 
-	  gdb_pretty_print_insn (gdbarch, uiout, &di, &dinsn, flags, stb);
+	  gdb_pretty_print_insn (gdbarch, uiout, &di, &dinsn, flags,
+				 stb.get ());
 	}
     }
 
diff --git a/gdb/regcache.c b/gdb/regcache.c
index b2b9524..33b15d8 100644
--- a/gdb/regcache.c
+++ b/gdb/regcache.c
@@ -1497,14 +1497,11 @@ regcache_print (char *args, enum regcache_dump_what what_to_dump)
     regcache_dump (get_current_regcache (), gdb_stdout, what_to_dump);
   else
     {
-      struct cleanup *cleanups;
-      struct ui_file *file = gdb_fopen (args, "w");
+      ui_file_up file (gdb_fopen (args, "w"));
 
       if (file == NULL)
 	perror_with_name (_("maintenance print architecture"));
-      cleanups = make_cleanup_ui_file_delete (file);
-      regcache_dump (get_current_regcache (), file, what_to_dump);
-      do_cleanups (cleanups);
+      regcache_dump (get_current_regcache (), file.get (), what_to_dump);
     }
 }
 
diff --git a/gdb/reggroups.c b/gdb/reggroups.c
index 693b378..c1c8999 100644
--- a/gdb/reggroups.c
+++ b/gdb/reggroups.c
@@ -270,14 +270,11 @@ maintenance_print_reggroups (char *args, int from_tty)
     reggroups_dump (gdbarch, gdb_stdout);
   else
     {
-      struct cleanup *cleanups;
-      struct ui_file *file = gdb_fopen (args, "w");
+      ui_file_up file (gdb_fopen (args, "w"));
 
       if (file == NULL)
 	perror_with_name (_("maintenance print reggroups"));
-      cleanups = make_cleanup_ui_file_delete (file);
-      reggroups_dump (gdbarch, file);
-      do_cleanups (cleanups);
+      reggroups_dump (gdbarch, file.get ());
     }
 }
 
diff --git a/gdb/remote.c b/gdb/remote.c
index 2f7954a..1bd8924 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -8661,15 +8661,11 @@ remote_send (char **buf,
 static std::string
 escape_buffer (const char *buf, int n)
 {
-  struct cleanup *old_chain;
-  struct ui_file *stb;
+  ui_file_up stb = mem_fileopen ();
 
-  stb = mem_fileopen ();
-  old_chain = make_cleanup_ui_file_delete (stb);
+  fputstrn_unfiltered (buf, n, '\\', stb.get ());
+  std::string str = ui_file_as_string (stb.get ());
 
-  fputstrn_unfiltered (buf, n, '\\', stb);
-  std::string str = ui_file_as_string (stb);
-  do_cleanups (old_chain);
   return str;
 }
 
diff --git a/gdb/rust-lang.c b/gdb/rust-lang.c
index 4445812..ad5406d 100644
--- a/gdb/rust-lang.c
+++ b/gdb/rust-lang.c
@@ -125,9 +125,7 @@ rust_get_disr_info (struct type *type, const gdb_byte *valaddr,
   int i;
   struct disr_info ret;
   struct type *disr_type;
-  struct ui_file *temp_file;
   struct value_print_options opts;
-  struct cleanup *cleanup;
   const char *name_segment;
 
   get_no_prettyformat_print_options (&opts);
@@ -229,17 +227,16 @@ rust_get_disr_info (struct type *type, const gdb_byte *valaddr,
   if (strcmp (TYPE_FIELD_NAME (disr_type, 0), "RUST$ENUM$DISR") != 0)
     error (_("Rust debug format has changed"));
 
-  temp_file = mem_fileopen ();
-  cleanup = make_cleanup_ui_file_delete (temp_file);
+  ui_file_up temp_file = mem_fileopen ();
   /* The first value of the first field (or any field)
      is the discriminant value.  */
   c_val_print (TYPE_FIELD_TYPE (disr_type, 0),
 	       (embedded_offset + TYPE_FIELD_BITPOS (type, 0) / 8
 		+ TYPE_FIELD_BITPOS (disr_type, 0) / 8),
-	       address, temp_file,
+	       address, temp_file.get (),
 	       0, val, &opts);
 
-  ret.name = ui_file_as_string (temp_file);
+  ret.name = ui_file_as_string (temp_file.get ());
   name_segment = rust_last_path_segment (ret.name.c_str ());
   if (name_segment != NULL)
     {
@@ -271,7 +268,6 @@ rust_get_disr_info (struct type *type, const gdb_byte *valaddr,
 	     TYPE_TAG_NAME (type), ret.name.c_str ());
     }
 
-  do_cleanups (cleanup);
   return ret;
 }
 
diff --git a/gdb/stack.c b/gdb/stack.c
index e00e297..27258c3 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -225,11 +225,9 @@ print_frame_arg (const struct frame_arg *arg)
 {
   struct ui_out *uiout = current_uiout;
   struct cleanup *old_chain;
-  struct ui_file *stb;
   const char *error_message = NULL;
 
-  stb = mem_fileopen ();
-  old_chain = make_cleanup_ui_file_delete (stb);
+  ui_file_up stb = mem_fileopen ();
 
   gdb_assert (!arg->val || !arg->error);
   gdb_assert (arg->entry_kind == print_entry_values_no
@@ -239,23 +237,23 @@ print_frame_arg (const struct frame_arg *arg)
 
   annotate_arg_begin ();
 
-  make_cleanup_ui_out_tuple_begin_end (uiout, NULL);
-  fprintf_symbol_filtered (stb, SYMBOL_PRINT_NAME (arg->sym),
+  old_chain = make_cleanup_ui_out_tuple_begin_end (uiout, NULL);
+  fprintf_symbol_filtered (stb.get (), SYMBOL_PRINT_NAME (arg->sym),
 			   SYMBOL_LANGUAGE (arg->sym), DMGL_PARAMS | DMGL_ANSI);
   if (arg->entry_kind == print_entry_values_compact)
     {
       /* It is OK to provide invalid MI-like stream as with
 	 PRINT_ENTRY_VALUE_COMPACT we never use MI.  */
-      fputs_filtered ("=", stb);
+      fputs_filtered ("=", stb.get ());
 
-      fprintf_symbol_filtered (stb, SYMBOL_PRINT_NAME (arg->sym),
+      fprintf_symbol_filtered (stb.get (), SYMBOL_PRINT_NAME (arg->sym),
 			       SYMBOL_LANGUAGE (arg->sym),
 			       DMGL_PARAMS | DMGL_ANSI);
     }
   if (arg->entry_kind == print_entry_values_only
       || arg->entry_kind == print_entry_values_compact)
-    fputs_filtered ("@entry", stb);
-  uiout->field_stream ("name", stb);
+    fputs_filtered ("@entry", stb.get ());
+  uiout->field_stream ("name", stb.get ());
   annotate_arg_name_end ();
   uiout->text ("=");
 
@@ -294,7 +292,7 @@ print_frame_arg (const struct frame_arg *arg)
 	      /* True in "summary" mode, false otherwise.  */
 	      opts.summary = !strcmp (print_frame_arguments, "scalars");
 
-	      common_val_print (arg->val, stb, 2, &opts, language);
+	      common_val_print (arg->val, stb.get (), 2, &opts, language);
 	    }
 	  CATCH (except, RETURN_MASK_ERROR)
 	    {
@@ -303,11 +301,11 @@ print_frame_arg (const struct frame_arg *arg)
 	  END_CATCH
 	}
       if (error_message != NULL)
-	fprintf_filtered (stb, _("<error reading variable: %s>"),
+	fprintf_filtered (stb.get (), _("<error reading variable: %s>"),
 			  error_message);
     }
 
-  uiout->field_stream ("value", stb);
+  uiout->field_stream ("value", stb.get ());
 
   /* Also invoke ui_out_tuple_end.  */
   do_cleanups (old_chain);
@@ -553,14 +551,9 @@ print_frame_args (struct symbol *func, struct frame_info *frame,
   long highest_offset = -1;
   /* Number of ints of arguments that we have printed so far.  */
   int args_printed = 0;
-  struct cleanup *old_chain;
-  struct ui_file *stb;
   /* True if we should print arguments, false otherwise.  */
   int print_args = strcmp (print_frame_arguments, "none");
 
-  stb = mem_fileopen ();
-  old_chain = make_cleanup_ui_file_delete (stb);
-
   if (func)
     {
       const struct block *b = SYMBOL_BLOCK_VALUE (func);
@@ -730,8 +723,6 @@ print_frame_args (struct symbol *func, struct frame_info *frame,
       print_frame_nameless_args (frame, start, num - args_printed,
 				 first, stream);
     }
-
-  do_cleanups (old_chain);
 }
 
 /* Set the current source and line to the location given by frame
@@ -1165,7 +1156,6 @@ print_frame (struct frame_info *frame, int print_level,
   struct ui_out *uiout = current_uiout;
   char *funname = NULL;
   enum language funlang = language_unknown;
-  struct ui_file *stb;
   struct cleanup *old_chain, *list_chain;
   struct value_print_options opts;
   struct symbol *func;
@@ -1174,11 +1164,10 @@ print_frame (struct frame_info *frame, int print_level,
 
   pc_p = get_frame_pc_if_available (frame, &pc);
 
-  stb = mem_fileopen ();
-  old_chain = make_cleanup_ui_file_delete (stb);
+  ui_file_up stb = mem_fileopen ();
 
   find_frame_funname (frame, &funname, &funlang, &func);
-  make_cleanup (xfree, funname);
+  old_chain = make_cleanup (xfree, funname);
 
   annotate_frame_begin (print_level ? frame_relative_level (frame) : 0,
 			gdbarch, pc);
@@ -1206,9 +1195,9 @@ print_frame (struct frame_info *frame, int print_level,
 	uiout->text (" in ");
       }
   annotate_frame_function_name ();
-  fprintf_symbol_filtered (stb, funname ? funname : "??",
+  fprintf_symbol_filtered (stb.get (), funname ? funname : "??",
 			   funlang, DMGL_ANSI);
-  uiout->field_stream ("func", stb);
+  uiout->field_stream ("func", stb.get ());
   uiout->wrap_hint ("   ");
   annotate_frame_args ();
       
diff --git a/gdb/top.c b/gdb/top.c
index f712bea..7b19fd8 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -693,7 +693,6 @@ execute_command (char *p, int from_tty)
 std::string
 execute_command_to_string (char *p, int from_tty)
 {
-  struct ui_file *str_file;
   struct cleanup *cleanup;
 
   /* GDB_STDOUT should be better already restored during these
@@ -702,27 +701,25 @@ execute_command_to_string (char *p, int from_tty)
 
   scoped_restore save_async = make_scoped_restore (&current_ui->async, 0);
 
-  str_file = mem_fileopen ();
+  ui_file_up str_file = mem_fileopen ();
 
-  make_cleanup_ui_file_delete (str_file);
-
-  current_uiout->redirect (str_file);
+  current_uiout->redirect (str_file.get ());
   make_cleanup_ui_out_redirect_pop (current_uiout);
 
   scoped_restore save_stdout
-    = make_scoped_restore (&gdb_stdout, str_file);
+    = make_scoped_restore (&gdb_stdout, str_file.get ());
   scoped_restore save_stderr
-    = make_scoped_restore (&gdb_stderr, str_file);
+    = make_scoped_restore (&gdb_stderr, str_file.get ());
   scoped_restore save_stdlog
-    = make_scoped_restore (&gdb_stdlog, str_file);
+    = make_scoped_restore (&gdb_stdlog, str_file.get ());
   scoped_restore save_stdtarg
-    = make_scoped_restore (&gdb_stdtarg, str_file);
+    = make_scoped_restore (&gdb_stdtarg, str_file.get ());
   scoped_restore save_stdtargerr
-    = make_scoped_restore (&gdb_stdtargerr, str_file);
+    = make_scoped_restore (&gdb_stdtargerr, str_file.get ());
 
   execute_command (p, from_tty);
 
-  std::string retval = ui_file_as_string (str_file);
+  std::string retval = ui_file_as_string (str_file.get ());
 
   do_cleanups (cleanup);
 
@@ -1569,24 +1566,18 @@ print_inferior_quit_action (struct inferior *inf, void *arg)
 int
 quit_confirm (void)
 {
-  struct ui_file *stb;
-  struct cleanup *old_chain;
-
   /* Don't even ask if we're only debugging a core file inferior.  */
   if (!have_live_inferiors ())
     return 1;
 
   /* Build the query string as a single string.  */
-  stb = mem_fileopen ();
-  old_chain = make_cleanup_ui_file_delete (stb);
+  ui_file_up stb = mem_fileopen ();
 
-  fprintf_filtered (stb, _("A debugging session is active.\n\n"));
-  iterate_over_inferiors (print_inferior_quit_action, stb);
-  fprintf_filtered (stb, _("\nQuit anyway? "));
+  fprintf_filtered (stb.get (), _("A debugging session is active.\n\n"));
+  iterate_over_inferiors (print_inferior_quit_action, stb.get ());
+  fprintf_filtered (stb.get (), _("\nQuit anyway? "));
 
-  std::string str = ui_file_as_string (stb);
-
-  do_cleanups (old_chain);
+  std::string str = ui_file_as_string (stb.get ());
 
   return query ("%s", str.c_str ());
 }
diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
index 18b569e..31e5208 100644
--- a/gdb/tracepoint.c
+++ b/gdb/tracepoint.c
@@ -1304,12 +1304,11 @@ collection_list::stringify ()
 void
 collection_list::append_exp (struct expression *exp)
 {
-  struct ui_file *tmp_stream = mem_fileopen ();
+  ui_file_up tmp_stream = mem_fileopen ();
 
-  print_expression (exp, tmp_stream);
+  print_expression (exp, tmp_stream.get ());
 
-  m_computed.push_back (ui_file_as_string (tmp_stream));
-  ui_file_delete (tmp_stream);
+  m_computed.push_back (ui_file_as_string (tmp_stream.get ()));
 }
 
 void
diff --git a/gdb/typeprint.c b/gdb/typeprint.c
index 56e993e..c73d4a7 100644
--- a/gdb/typeprint.c
+++ b/gdb/typeprint.c
@@ -372,24 +372,19 @@ std::string
 type_to_string (struct type *type)
 {
   std::string s;
-  struct ui_file *stb;
-  struct cleanup *old_chain;
 
-  stb = mem_fileopen ();
-  old_chain = make_cleanup_ui_file_delete (stb);
+  ui_file_up stb = mem_fileopen ();
 
   TRY
     {
-      type_print (type, "", stb, -1);
-      s = ui_file_as_string (stb);
+      type_print (type, "", stb.get (), -1);
+      s = ui_file_as_string (stb.get ());
     }
   CATCH (except, RETURN_MASK_ALL)
     {
     }
   END_CATCH
 
-  do_cleanups (old_chain);
-
   return s;
 }
 
diff --git a/gdb/ui-file.c b/gdb/ui-file.c
index 3dce511..621b71b 100644
--- a/gdb/ui-file.c
+++ b/gdb/ui-file.c
@@ -445,10 +445,10 @@ mem_file_delete (struct ui_file *file)
   xfree (stream);
 }
 
-struct ui_file *
+ui_file_up
 mem_fileopen (void)
 {
-  return mem_file_new ();
+  return ui_file_up (mem_file_new ());
 }
 
 static void
diff --git a/gdb/ui-file.h b/gdb/ui-file.h
index 4ad3940..14b09c8 100644
--- a/gdb/ui-file.h
+++ b/gdb/ui-file.h
@@ -23,6 +23,7 @@ struct obstack;
 struct ui_file;
 
 #include <string>
+#include <memory>
 
 /* Create a generic ui_file object with null methods.  */
 
@@ -93,6 +94,18 @@ extern void gdb_flush (struct ui_file *);
 
 extern void ui_file_delete (struct ui_file *stream);
 
+/* A deleter that calls ui_file_delete.  */
+struct ui_file_deleter
+{
+  void operator() (struct ui_file *ptr) const
+  {
+    ui_file_delete (ptr);
+  }
+};
+
+/* A unique pointer for ui_file objects.  */
+typedef std::unique_ptr<ui_file, ui_file_deleter> ui_file_up;
+
 extern void ui_file_rewind (struct ui_file *stream);
 
 extern int ui_file_isatty (struct ui_file *);
@@ -134,9 +147,7 @@ extern int ui_file_fseek (struct ui_file *file, long offset, int whence);
 
 /* Create/open a memory based file.  Can be used as a scratch buffer
    for collecting output.  */
-extern struct ui_file *mem_fileopen (void);
-
-
+extern ui_file_up mem_fileopen (void);
 
 /* Open/create a STDIO based UI_FILE using the already open FILE.  */
 extern struct ui_file *stdio_fileopen (FILE *file);
@@ -155,4 +166,5 @@ extern struct ui_file *tee_file_new (struct ui_file *one,
 				     int close_one,
 				     struct ui_file *two,
 				     int close_two);
+
 #endif
diff --git a/gdb/varobj.c b/gdb/varobj.c
index 30dfb86..6e76619 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -2400,8 +2400,6 @@ varobj_value_get_print_value (struct value *value,
 			      enum varobj_display_formats format,
 			      const struct varobj *var)
 {
-  struct ui_file *stb;
-  struct cleanup *old_chain;
   struct value_print_options opts;
   struct type *type = NULL;
   long len = 0;
@@ -2413,8 +2411,7 @@ varobj_value_get_print_value (struct value *value,
   if (value == NULL)
     return std::string ();
 
-  stb = mem_fileopen ();
-  old_chain = make_cleanup_ui_file_delete (stb);
+  ui_file_up stb = mem_fileopen ();
 
   std::string thevalue;
 
@@ -2430,10 +2427,7 @@ varobj_value_get_print_value (struct value *value,
 	  /* First check to see if we have any children at all.  If so,
 	     we simply return {...}.  */
 	  if (dynamic_varobj_has_child_method (var))
-	    {
-	      do_cleanups (old_chain);
-	      return xstrdup ("{...}");
-	    }
+	    return xstrdup ("{...}");
 
 	  if (PyObject_HasAttr (value_formatter, gdbpy_to_string_cst))
 	    {
@@ -2441,7 +2435,7 @@ varobj_value_get_print_value (struct value *value,
 
 	      gdbpy_ref output (apply_varobj_pretty_printer (value_formatter,
 							     &replacement,
-							     stb));
+							     stb.get ()));
 
 	      /* If we have string like output ...  */
 	      if (output != NULL)
@@ -2484,10 +2478,7 @@ varobj_value_get_print_value (struct value *value,
 			  type = builtin_type (gdbarch)->builtin_char;
 
 			  if (!string_print)
-			    {
-			      do_cleanups (old_chain);
-			      return thevalue;
-			    }
+			    return thevalue;
 			}
 		      else
 			gdbpy_print_stack ();
@@ -2507,20 +2498,17 @@ varobj_value_get_print_value (struct value *value,
 
   /* If the THEVALUE has contents, it is a regular string.  */
   if (!thevalue.empty ())
-    LA_PRINT_STRING (stb, type, (gdb_byte *) thevalue.c_str (),
+    LA_PRINT_STRING (stb.get (), type, (gdb_byte *) thevalue.c_str (),
 		     len, encoding.get (), 0, &opts);
   else if (string_print)
     /* Otherwise, if string_print is set, and it is not a regular
        string, it is a lazy string.  */
-    val_print_string (type, encoding.get (), str_addr, len, stb, &opts);
+    val_print_string (type, encoding.get (), str_addr, len, stb.get (), &opts);
   else
     /* All other cases.  */
-    common_val_print (value, stb, 0, &opts, current_language);
-
-  thevalue = ui_file_as_string (stb);
+    common_val_print (value, stb.get (), 0, &opts, current_language);
 
-  do_cleanups (old_chain);
-  return thevalue;
+  return ui_file_as_string (stb.get ());
 }
 
 int
diff --git a/gdb/xtensa-tdep.c b/gdb/xtensa-tdep.c
index 978b13a..6bd6f26 100644
--- a/gdb/xtensa-tdep.c
+++ b/gdb/xtensa-tdep.c
@@ -3063,45 +3063,41 @@ xtensa_skip_prologue (struct gdbarch *gdbarch, CORE_ADDR start_pc)
 static void
 xtensa_verify_config (struct gdbarch *gdbarch)
 {
-  struct ui_file *log;
-  struct cleanup *cleanups;
   struct gdbarch_tdep *tdep;
 
   tdep = gdbarch_tdep (gdbarch);
-  log = mem_fileopen ();
-  cleanups = make_cleanup_ui_file_delete (log);
+  ui_file_up log = mem_fileopen ();
 
   /* Verify that we got a reasonable number of AREGS.  */
   if ((tdep->num_aregs & -tdep->num_aregs) != tdep->num_aregs)
-    fprintf_unfiltered (log, _("\
+    fprintf_unfiltered (log.get (), _("\
 \n\tnum_aregs: Number of AR registers (%d) is not a power of two!"),
 			tdep->num_aregs);
 
   /* Verify that certain registers exist.  */
 
   if (tdep->pc_regnum == -1)
-    fprintf_unfiltered (log, _("\n\tpc_regnum: No PC register"));
+    fprintf_unfiltered (log.get (), _("\n\tpc_regnum: No PC register"));
   if (tdep->isa_use_exceptions && tdep->ps_regnum == -1)
-    fprintf_unfiltered (log, _("\n\tps_regnum: No PS register"));
+    fprintf_unfiltered (log.get (), _("\n\tps_regnum: No PS register"));
 
   if (tdep->isa_use_windowed_registers)
     {
       if (tdep->wb_regnum == -1)
-	fprintf_unfiltered (log, _("\n\twb_regnum: No WB register"));
+	fprintf_unfiltered (log.get (), _("\n\twb_regnum: No WB register"));
       if (tdep->ws_regnum == -1)
-	fprintf_unfiltered (log, _("\n\tws_regnum: No WS register"));
+	fprintf_unfiltered (log.get (), _("\n\tws_regnum: No WS register"));
       if (tdep->ar_base == -1)
-	fprintf_unfiltered (log, _("\n\tar_base: No AR registers"));
+	fprintf_unfiltered (log.get (), _("\n\tar_base: No AR registers"));
     }
 
   if (tdep->a0_base == -1)
-    fprintf_unfiltered (log, _("\n\ta0_base: No Ax registers"));
+    fprintf_unfiltered (log.get (), _("\n\ta0_base: No Ax registers"));
 
-  std::string buf = ui_file_as_string (log);
+  std::string buf = ui_file_as_string (log.get ());
   if (!buf.empty ())
     internal_error (__FILE__, __LINE__,
 		    _("the following are invalid: %s"), buf.c_str ());
-  do_cleanups (cleanups);
 }
 
 
-- 
2.9.3

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

* [RFA 1/5] Remove some ui_out-related cleanups from Python
  2017-01-15 13:43 [RFA 0/5] more cleanup removal in Python Tom Tromey
  2017-01-15 13:43 ` [RFA 3/5] Introduce gdbpy_subclass and use it to simplify some logic Tom Tromey
  2017-01-15 13:43 ` [RFA 4/5] Change one more spot to use gdbpy_ref Tom Tromey
@ 2017-01-15 13:43 ` Tom Tromey
  2017-01-15 21:52   ` Simon Marchi
  2017-01-16 11:19   ` Trevor Saunders
  2017-01-15 13:43 ` [RFA 2/5] Introduce ui_file_up and use it to remove cleanups Tom Tromey
  2017-01-15 13:43 ` [RFA 5/5] Remove some gotos from Python Tom Tromey
  4 siblings, 2 replies; 30+ messages in thread
From: Tom Tromey @ 2017-01-15 13:43 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This patch introduces a bit of infrastructure -- namely, a minimal
std::optional analogue called gdb::optional, and an RAII template
class that works like make_cleanup_ui_out_tuple_begin_end or
make_cleanup_ui_out_list_begin_end -- and then uses these in the
Python code.  This removes a number of cleanups and generally
simplifies this code.

std::optional is only available in C++17.  Normally I would have had
this code check __cplusplus, but my gcc apparently isn't new enough to
find <optional>, even with -std=c++1z; so, because I could not test
it, the patch does not do this.

2017-01-15  Tom Tromey  <tom@tromey.com>

	* ui-out.h (ui_out_emit_type): New class.
	(ui_out_emit_tuple, ui_out_emit_list): New typedefs.
	* python/py-framefilter.c (py_print_single_arg): Use gdb::optional
	and ui_out_emit_tuple.
	(enumerate_locals): Likewise.
	(py_mi_print_variables, py_print_locals, py_print_args): Use
	ui_out_emit_list.
	(py_print_frame): Use gdb::optional, ui_out_emit_tuple,
	ui_out_emit_list.
	* common/gdb_option.h: New file.
---
 gdb/ChangeLog               |  13 +++
 gdb/common/gdb_option.h     |  87 ++++++++++++++++++
 gdb/python/py-framefilter.c | 213 ++++++++++++--------------------------------
 gdb/ui-out.h                |  33 +++++++
 4 files changed, 189 insertions(+), 157 deletions(-)
 create mode 100644 gdb/common/gdb_option.h

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 39d66b8..8a2848f 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,16 @@
+2017-01-15  Tom Tromey  <tom@tromey.com>
+
+	* ui-out.h (ui_out_emit_type): New class.
+	(ui_out_emit_tuple, ui_out_emit_list): New typedefs.
+	* python/py-framefilter.c (py_print_single_arg): Use gdb::optional
+	and ui_out_emit_tuple.
+	(enumerate_locals): Likewise.
+	(py_mi_print_variables, py_print_locals, py_print_args): Use
+	ui_out_emit_list.
+	(py_print_frame): Use gdb::optional, ui_out_emit_tuple,
+	ui_out_emit_list.
+	* common/gdb_option.h: New file.
+
 2017-01-13  Yao Qi  <yao.qi@linaro.org>
 
 	* remote.c (REMOTE_DEBUG_MAX_CHAR): New macro.
diff --git a/gdb/common/gdb_option.h b/gdb/common/gdb_option.h
new file mode 100644
index 0000000..e2ac107
--- /dev/null
+++ b/gdb/common/gdb_option.h
@@ -0,0 +1,87 @@
+/* An optional object.
+
+   Copyright (C) 2017 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 GDB_OPTIONAL_H
+#define GDB_OPTIONAL_H
+
+namespace gdb
+{
+
+/* This class attempts to be a compatible subset of std::optional,
+   which is slated to be available in C++17.  This class optionally
+   holds an object of some type -- by default it is constructed not
+   holding an object, but later the object can be "emplaced".  This is
+   similar to using std::unique_ptr, but stack allocation is
+   guaranteed.  */
+template<typename T>
+class optional
+{
+public:
+
+  optional ()
+    : m_instantiated (false)
+  {
+  }
+
+  ~optional ()
+  {
+    if (m_instantiated)
+      destroy ();
+  }
+
+  /* These aren't deleted in std::optional, but it was simpler to
+     delete them here, because currently the users of this class don't
+     need them, and making them depend on the definition of T is
+     somewhat complicated.  */
+  optional (const optional &other) = delete;
+  optional<T> &operator= (const optional &other) = delete;
+
+  template<typename... Args>
+  void emplace (Args &&... args)
+  {
+    if (m_instantiated)
+      destroy ();
+    new (&m_item) T (std::forward<Args>(args)...);
+    m_instantiated = true;
+  }
+
+private:
+
+  /* Destroy the object.  */
+  void destroy ()
+  {
+    gdb_assert (m_instantiated);
+    m_instantiated = false;
+    m_item.~T ();
+  }
+
+  /* True if the object was ever emplaced.  */
+  bool m_instantiated;
+
+  /* The object.  */
+  union
+  {
+    struct { } m_dummy;
+    T m_item;
+  };
+};
+
+}
+
+#endif /* GDB_OPTIONAL_H */
diff --git a/gdb/python/py-framefilter.c b/gdb/python/py-framefilter.c
index bdd9911..ae5197a 100644
--- a/gdb/python/py-framefilter.c
+++ b/gdb/python/py-framefilter.c
@@ -31,6 +31,7 @@
 #include "mi/mi-cmds.h"
 #include "python-internal.h"
 #include "py-ref.h"
+#include "common/gdb_option.h"
 
 enum mi_print_types
 {
@@ -373,7 +374,7 @@ py_print_single_arg (struct ui_out *out,
 
   TRY
     {
-      struct cleanup *cleanups = make_cleanup (null_cleanup, NULL);
+      gdb::optional<ui_out_emit_tuple> maybe_tuple;
 
       /*  MI has varying rules for tuples, but generally if there is only
       one element in each item in the list, do not start a tuple.  The
@@ -384,7 +385,7 @@ py_print_single_arg (struct ui_out *out,
       if (out->is_mi_like_p ())
 	{
 	  if (print_args_field || args_type != NO_VALUES)
-	    make_cleanup_ui_out_tuple_begin_end (out, NULL);
+	    maybe_tuple.emplace (out, nullptr);
 	}
 
       annotate_arg_begin ();
@@ -394,9 +395,10 @@ py_print_single_arg (struct ui_out *out,
       if (fa != NULL)
 	{
 	  struct ui_file *stb;
+	  struct cleanup *cleanup;
 
 	  stb = mem_fileopen ();
-	  make_cleanup_ui_file_delete (stb);
+	  cleanup = make_cleanup_ui_file_delete (stb);
 	  fprintf_symbol_filtered (stb, SYMBOL_PRINT_NAME (fa->sym),
 				   SYMBOL_LANGUAGE (fa->sym),
 				   DMGL_PARAMS | DMGL_ANSI);
@@ -414,6 +416,7 @@ py_print_single_arg (struct ui_out *out,
 	      fputs_filtered ("@entry", stb);
 	    }
 	  out->field_stream ("name", stb);
+	  do_cleanups (cleanup);
 	}
       else
 	/* Otherwise, just output the name.  */
@@ -433,10 +436,7 @@ py_print_single_arg (struct ui_out *out,
       if (args_type == MI_PRINT_SIMPLE_VALUES && val != NULL)
 	{
 	  if (py_print_type (out, val) == EXT_LANG_BT_ERROR)
-	    {
-	      retval = EXT_LANG_BT_ERROR;
-	      do_cleanups (cleanups);
-	    }
+	    retval = EXT_LANG_BT_ERROR;
 	}
 
       if (retval != EXT_LANG_BT_ERROR)
@@ -466,8 +466,6 @@ py_print_single_arg (struct ui_out *out,
 		    retval = EXT_LANG_BT_ERROR;
 		}
 	    }
-
-	  do_cleanups (cleanups);
 	}
     }
   CATCH (except, RETURN_MASK_ERROR)
@@ -707,35 +705,24 @@ enumerate_locals (PyObject *iter,
       struct symbol *sym;
       struct block *sym_block;
       int local_indent = 8 + (8 * indent);
-      struct cleanup *locals_cleanups;
+      gdb::optional<ui_out_emit_tuple> tuple;
 
       gdbpy_ref item (PyIter_Next (iter));
       if (item == NULL)
 	break;
 
-      locals_cleanups = make_cleanup (null_cleanup, NULL);
-
       success = extract_sym (item.get (), &sym_name, &sym, &sym_block,
 			     &language);
       if (success == EXT_LANG_BT_ERROR)
-	{
-	  do_cleanups (locals_cleanups);
-	  goto error;
-	}
+	return EXT_LANG_BT_ERROR;
 
       success = extract_value (item.get (), &val);
       if (success == EXT_LANG_BT_ERROR)
-	{
-	  do_cleanups (locals_cleanups);
-	  goto error;
-	}
+	return EXT_LANG_BT_ERROR;
 
       if (sym != NULL && out->is_mi_like_p ()
 	  && ! mi_should_print (sym, MI_PRINT_LOCALS))
-	{
-	  do_cleanups (locals_cleanups);
-	  continue;
-	}
+	continue;
 
       /* If the object did not provide a value, read it.  */
       if (val == NULL)
@@ -747,8 +734,7 @@ enumerate_locals (PyObject *iter,
 	  CATCH (except, RETURN_MASK_ERROR)
 	    {
 	      gdbpy_convert_exception (except);
-	      do_cleanups (locals_cleanups);
-	      goto error;
+	      return EXT_LANG_BT_ERROR;
 	    }
 	  END_CATCH
 	}
@@ -759,7 +745,7 @@ enumerate_locals (PyObject *iter,
       if (out->is_mi_like_p ())
 	{
 	  if (print_args_field || args_type != NO_VALUES)
-	    make_cleanup_ui_out_tuple_begin_end (out, NULL);
+	    tuple.emplace (out, nullptr);
 	}
       TRY
 	{
@@ -777,18 +763,14 @@ enumerate_locals (PyObject *iter,
       CATCH (except, RETURN_MASK_ERROR)
 	{
 	  gdbpy_convert_exception (except);
-	  do_cleanups (locals_cleanups);
-	  goto error;
+	  return EXT_LANG_BT_ERROR;
 	}
       END_CATCH
 
       if (args_type == MI_PRINT_SIMPLE_VALUES)
 	{
 	  if (py_print_type (out, val) == EXT_LANG_BT_ERROR)
-	    {
-	      do_cleanups (locals_cleanups);
-	      goto error;
-	    }
+	    return EXT_LANG_BT_ERROR;
 	}
 
       /* CLI always prints values for locals.  MI uses the
@@ -799,10 +781,7 @@ enumerate_locals (PyObject *iter,
 
 	  if (py_print_value (out, val, &opts, val_indent, args_type,
 			      language) == EXT_LANG_BT_ERROR)
-	    {
-	      do_cleanups (locals_cleanups);
-	      goto error;
-	    }
+	    return EXT_LANG_BT_ERROR;
 	}
       else
 	{
@@ -810,15 +789,10 @@ enumerate_locals (PyObject *iter,
 	    {
 	      if (py_print_value (out, val, &opts, 0, args_type,
 				  language) == EXT_LANG_BT_ERROR)
-		{
-		  do_cleanups (locals_cleanups);
-		  goto error;
-		}
+		return EXT_LANG_BT_ERROR;
 	    }
 	}
 
-      do_cleanups (locals_cleanups);
-
       TRY
 	{
 	  out->text ("\n");
@@ -826,7 +800,7 @@ enumerate_locals (PyObject *iter,
       CATCH (except, RETURN_MASK_ERROR)
 	{
 	  gdbpy_convert_exception (except);
-	  goto error;
+	  return EXT_LANG_BT_ERROR;
 	}
       END_CATCH
     }
@@ -834,7 +808,6 @@ enumerate_locals (PyObject *iter,
   if (!PyErr_Occurred ())
     return EXT_LANG_BT_OK;
 
- error:
   return EXT_LANG_BT_ERROR;
 }
 
@@ -847,8 +820,6 @@ py_mi_print_variables (PyObject *filter, struct ui_out *out,
 		       enum ext_lang_frame_args args_type,
 		       struct frame_info *frame)
 {
-  struct cleanup *old_chain;
-
   gdbpy_ref args_iter (get_py_iter_from_func (filter, "frame_args"));
   if (args_iter == NULL)
     return EXT_LANG_BT_ERROR;
@@ -857,24 +828,19 @@ py_mi_print_variables (PyObject *filter, struct ui_out *out,
   if (locals_iter == NULL)
     return EXT_LANG_BT_ERROR;
 
-  old_chain = make_cleanup_ui_out_list_begin_end (out, "variables");
+  ui_out_emit_list list_emitter (out, "variables");
 
-  if (args_iter != Py_None)
-    if (enumerate_args (args_iter.get (), out, args_type, 1, frame)
-	== EXT_LANG_BT_ERROR)
-      goto error;
+  if (args_iter != Py_None
+      && (enumerate_args (args_iter.get (), out, args_type, 1, frame)
+	  == EXT_LANG_BT_ERROR))
+    return EXT_LANG_BT_ERROR;
 
-  if (locals_iter != Py_None)
-    if (enumerate_locals (locals_iter.get (), out, 1, args_type, 1, frame)
-	== EXT_LANG_BT_ERROR)
-      goto error;
+  if (locals_iter != Py_None
+      && (enumerate_locals (locals_iter.get (), out, 1, args_type, 1, frame)
+	  == EXT_LANG_BT_ERROR))
+    return EXT_LANG_BT_ERROR;
 
-  do_cleanups (old_chain);
   return EXT_LANG_BT_OK;
-
- error:
-  do_cleanups (old_chain);
-  return EXT_LANG_BT_ERROR;
 }
 
 /* Helper function for printing locals.  This function largely just
@@ -888,25 +854,18 @@ py_print_locals (PyObject *filter,
 		 int indent,
 		 struct frame_info *frame)
 {
-  struct cleanup *old_chain;
-
   gdbpy_ref locals_iter (get_py_iter_from_func (filter, "frame_locals"));
   if (locals_iter == NULL)
     return EXT_LANG_BT_ERROR;
 
-  old_chain = make_cleanup_ui_out_list_begin_end (out, "locals");
+  ui_out_emit_list list_emitter (out, "locals");
 
-  if (locals_iter != Py_None)
-    if (enumerate_locals (locals_iter.get (), out, indent, args_type,
-			  0, frame) == EXT_LANG_BT_ERROR)
-      goto locals_error;
+  if (locals_iter != Py_None
+      && (enumerate_locals (locals_iter.get (), out, indent, args_type,
+			    0, frame) == EXT_LANG_BT_ERROR))
+    return EXT_LANG_BT_ERROR;
 
-  do_cleanups (old_chain);
   return EXT_LANG_BT_OK;
-
- locals_error:
-  do_cleanups (old_chain);
-  return EXT_LANG_BT_ERROR;
 }
 
 /* Helper function for printing frame arguments.  This function
@@ -920,13 +879,11 @@ py_print_args (PyObject *filter,
 	       enum ext_lang_frame_args args_type,
 	       struct frame_info *frame)
 {
-  struct cleanup *old_chain;
-
   gdbpy_ref args_iter (get_py_iter_from_func (filter, "frame_args"));
   if (args_iter == NULL)
     return EXT_LANG_BT_ERROR;
 
-  old_chain = make_cleanup_ui_out_list_begin_end (out, "args");
+  ui_out_emit_list list_emitter (out, "args");
 
   TRY
     {
@@ -937,14 +894,14 @@ py_print_args (PyObject *filter,
   CATCH (except, RETURN_MASK_ALL)
     {
       gdbpy_convert_exception (except);
-      goto args_error;
+      return EXT_LANG_BT_ERROR;
     }
   END_CATCH
 
-  if (args_iter != Py_None)
-    if (enumerate_args (args_iter.get (), out, args_type, 0, frame)
-	== EXT_LANG_BT_ERROR)
-      goto args_error;
+  if (args_iter != Py_None
+      && (enumerate_args (args_iter.get (), out, args_type, 0, frame)
+	  == EXT_LANG_BT_ERROR))
+    return EXT_LANG_BT_ERROR;
 
   TRY
     {
@@ -954,16 +911,11 @@ py_print_args (PyObject *filter,
   CATCH (except, RETURN_MASK_ALL)
     {
       gdbpy_convert_exception (except);
-      goto args_error;
+      return EXT_LANG_BT_ERROR;
     }
   END_CATCH
 
-  do_cleanups (old_chain);
   return EXT_LANG_BT_OK;
-
- args_error:
-  do_cleanups (old_chain);
-  return EXT_LANG_BT_ERROR;
 }
 
 /*  Print a single frame to the designated output stream, detecting
@@ -990,7 +942,6 @@ py_print_frame (PyObject *filter, int flags,
   CORE_ADDR address = 0;
   struct gdbarch *gdbarch = NULL;
   struct frame_info *frame = NULL;
-  struct cleanup *cleanup_stack;
   struct value_print_options opts;
   int print_level, print_frame_info, print_args, print_locals;
   gdb::unique_xmalloc_ptr<char> function_to_free;
@@ -1034,12 +985,12 @@ py_print_frame (PyObject *filter, int flags,
       return EXT_LANG_BT_COMPLETED;
     }
 
-  cleanup_stack = make_cleanup (null_cleanup, NULL);
+  gdb::optional<ui_out_emit_tuple> tuple;
 
   /* -stack-list-locals does not require a
      wrapping frame attribute.  */
   if (print_frame_info || (print_args && ! print_locals))
-    make_cleanup_ui_out_tuple_begin_end (out, "frame");
+    tuple.emplace (out, "frame");
 
   if (print_frame_info)
     {
@@ -1054,7 +1005,6 @@ py_print_frame (PyObject *filter, int flags,
 	  CATCH (except, RETURN_MASK_ERROR)
 	    {
 	      gdbpy_convert_exception (except);
-	      do_cleanups (cleanup_stack);
 	      return EXT_LANG_BT_ERROR;
 	    }
 	  END_CATCH
@@ -1067,18 +1017,12 @@ py_print_frame (PyObject *filter, int flags,
 	  gdbpy_ref paddr (PyObject_CallMethod (filter, "address", NULL));
 
 	  if (paddr == NULL)
-	    {
-	      do_cleanups (cleanup_stack);
-	      return EXT_LANG_BT_ERROR;
-	    }
+	    return EXT_LANG_BT_ERROR;
 
 	  if (paddr != Py_None)
 	    {
 	      if (get_addr_from_python (paddr.get (), &address) < 0)
-		{
-		  do_cleanups (cleanup_stack);
-		  return EXT_LANG_BT_ERROR;
-		}
+		return EXT_LANG_BT_ERROR;
 
 	      has_addr = 1;
 	    }
@@ -1117,7 +1061,6 @@ py_print_frame (PyObject *filter, int flags,
       CATCH (except, RETURN_MASK_ERROR)
 	{
 	  gdbpy_convert_exception (except);
-	  do_cleanups (cleanup_stack);
 	  return EXT_LANG_BT_ERROR;
 	}
       END_CATCH
@@ -1139,7 +1082,6 @@ py_print_frame (PyObject *filter, int flags,
 	  CATCH (except, RETURN_MASK_ERROR)
 	    {
 	      gdbpy_convert_exception (except);
-	      do_cleanups (cleanup_stack);
 	      return EXT_LANG_BT_ERROR;
 	    }
 	  END_CATCH
@@ -1152,20 +1094,14 @@ py_print_frame (PyObject *filter, int flags,
 	  const char *function = NULL;
 
 	  if (py_func == NULL)
-	    {
-	      do_cleanups (cleanup_stack);
-	      return EXT_LANG_BT_ERROR;
-	    }
+	    return EXT_LANG_BT_ERROR;
 
 	  if (gdbpy_is_string (py_func.get ()))
 	    {
 	      function_to_free = python_string_to_host_string (py_func.get ());
 
 	      if (function_to_free == NULL)
-		{
-		  do_cleanups (cleanup_stack);
-		  return EXT_LANG_BT_ERROR;
-		}
+		return EXT_LANG_BT_ERROR;
 
 	      function = function_to_free.get ();
 	    }
@@ -1175,10 +1111,7 @@ py_print_frame (PyObject *filter, int flags,
 	      struct bound_minimal_symbol msymbol;
 
 	      if (get_addr_from_python (py_func.get (), &addr) < 0)
-		{
-		  do_cleanups (cleanup_stack);
-		  return EXT_LANG_BT_ERROR;
-		}
+		return EXT_LANG_BT_ERROR;
 
 	      msymbol = lookup_minimal_symbol_by_pc (addr);
 	      if (msymbol.minsym != NULL)
@@ -1189,7 +1122,6 @@ py_print_frame (PyObject *filter, int flags,
 	      PyErr_SetString (PyExc_RuntimeError,
 			       _("FrameDecorator.function: expecting a " \
 				 "String, integer or None."));
-	      do_cleanups (cleanup_stack);
 	      return EXT_LANG_BT_ERROR;
 	    }
 
@@ -1204,7 +1136,6 @@ py_print_frame (PyObject *filter, int flags,
 	  CATCH (except, RETURN_MASK_ERROR)
 	    {
 	      gdbpy_convert_exception (except);
-	      do_cleanups (cleanup_stack);
 	      return EXT_LANG_BT_ERROR;
 	    }
 	  END_CATCH
@@ -1217,10 +1148,7 @@ py_print_frame (PyObject *filter, int flags,
   if (print_args)
     {
       if (py_print_args (filter, out, args_type, frame) == EXT_LANG_BT_ERROR)
-	{
-	  do_cleanups (cleanup_stack);
-	  return EXT_LANG_BT_ERROR;
-	}
+	return EXT_LANG_BT_ERROR;
     }
 
   /* File name/source/line number information.  */
@@ -1233,7 +1161,6 @@ py_print_frame (PyObject *filter, int flags,
       CATCH (except, RETURN_MASK_ERROR)
 	{
 	  gdbpy_convert_exception (except);
-	  do_cleanups (cleanup_stack);
 	  return EXT_LANG_BT_ERROR;
 	}
       END_CATCH
@@ -1243,10 +1170,7 @@ py_print_frame (PyObject *filter, int flags,
 	  gdbpy_ref py_fn (PyObject_CallMethod (filter, "filename", NULL));
 
 	  if (py_fn == NULL)
-	    {
-	      do_cleanups (cleanup_stack);
-	      return EXT_LANG_BT_ERROR;
-	    }
+	    return EXT_LANG_BT_ERROR;
 
 	  if (py_fn != Py_None)
 	    {
@@ -1254,10 +1178,7 @@ py_print_frame (PyObject *filter, int flags,
 		filename (python_string_to_host_string (py_fn.get ()));
 
 	      if (filename == NULL)
-		{
-		  do_cleanups (cleanup_stack);
-		  return EXT_LANG_BT_ERROR;
-		}
+		return EXT_LANG_BT_ERROR;
 
 	      TRY
 		{
@@ -1270,7 +1191,6 @@ py_print_frame (PyObject *filter, int flags,
 	      CATCH (except, RETURN_MASK_ERROR)
 		{
 		  gdbpy_convert_exception (except);
-		  do_cleanups (cleanup_stack);
 		  return EXT_LANG_BT_ERROR;
 		}
 	      END_CATCH
@@ -1283,19 +1203,13 @@ py_print_frame (PyObject *filter, int flags,
 	  int line;
 
 	  if (py_line == NULL)
-	    {
-	      do_cleanups (cleanup_stack);
-	      return EXT_LANG_BT_ERROR;
-	    }
+	    return EXT_LANG_BT_ERROR;
 
 	  if (py_line != Py_None)
 	    {
 	      line = PyLong_AsLong (py_line.get ());
 	      if (PyErr_Occurred ())
-		{
-		  do_cleanups (cleanup_stack);
-		  return EXT_LANG_BT_ERROR;
-		}
+		return EXT_LANG_BT_ERROR;
 
 	      TRY
 		{
@@ -1306,7 +1220,6 @@ py_print_frame (PyObject *filter, int flags,
 	      CATCH (except, RETURN_MASK_ERROR)
 		{
 		  gdbpy_convert_exception (except);
-		  do_cleanups (cleanup_stack);
 		  return EXT_LANG_BT_ERROR;
 		}
 	      END_CATCH
@@ -1326,7 +1239,6 @@ py_print_frame (PyObject *filter, int flags,
       CATCH (except, RETURN_MASK_ERROR)
 	{
 	  gdbpy_convert_exception (except);
-	  do_cleanups (cleanup_stack);
 	  return EXT_LANG_BT_ERROR;
 	}
       END_CATCH
@@ -1336,26 +1248,20 @@ py_print_frame (PyObject *filter, int flags,
     {
       if (py_print_locals (filter, out, args_type, indent,
 			   frame) == EXT_LANG_BT_ERROR)
-	{
-	  do_cleanups (cleanup_stack);
-	  return EXT_LANG_BT_ERROR;
-	}
+	return EXT_LANG_BT_ERROR;
     }
 
   {
     /* Finally recursively print elided frames, if any.  */
     gdbpy_ref elided (get_py_iter_from_func (filter, "elided"));
     if (elided == NULL)
-      {
-	do_cleanups (cleanup_stack);
-	return EXT_LANG_BT_ERROR;
-      }
+      return EXT_LANG_BT_ERROR;
 
     if (elided != Py_None)
       {
 	PyObject *item;
 
-	make_cleanup_ui_out_list_begin_end (out, "children");
+	ui_out_emit_list inner_list_emiter (out, "children");
 
 	if (! out->is_mi_like_p ())
 	  indent++;
@@ -1370,20 +1276,13 @@ py_print_frame (PyObject *filter, int flags,
 							      levels_printed);
 
 	    if (success == EXT_LANG_BT_ERROR)
-	      {
-		do_cleanups (cleanup_stack);
-		return EXT_LANG_BT_ERROR;
-	      }
+	      return EXT_LANG_BT_ERROR;
 	  }
 	if (item == NULL && PyErr_Occurred ())
-	  {
-	    do_cleanups (cleanup_stack);
-	    return EXT_LANG_BT_ERROR;
-	  }
+	  return EXT_LANG_BT_ERROR;
       }
   }
 
-  do_cleanups (cleanup_stack);
   return EXT_LANG_BT_COMPLETED;
 }
 
diff --git a/gdb/ui-out.h b/gdb/ui-out.h
index b8bea97..7d5a1fc 100644
--- a/gdb/ui-out.h
+++ b/gdb/ui-out.h
@@ -187,4 +187,37 @@ class ui_out
   ui_out_level *current_level () const;
 };
 
+/* This is similar to make_cleanup_ui_out_tuple_begin_end and
+   make_cleanup_ui_out_list_begin_end, but written as an RAII template
+   class.  It takes the ui_out_type as a template parameter.  Normally
+   this is used via the typedefs ui_out_emit_tuple and
+   ui_out_emit_list.  */
+template<ui_out_type Type>
+class ui_out_emit_type
+{
+public:
+
+  ui_out_emit_type (struct ui_out *uiout, const char *id)
+    : m_uiout (uiout)
+  {
+    uiout->begin (Type, id);
+  }
+
+  ~ui_out_emit_type ()
+  {
+    m_uiout->end (Type);
+  }
+
+  ui_out_emit_type (const ui_out_emit_type<Type> &) = delete;
+  ui_out_emit_type<Type> &operator= (const ui_out_emit_type<Type> &)
+      = delete;
+
+private:
+
+  struct ui_out *m_uiout;
+};
+
+typedef ui_out_emit_type<ui_out_type_tuple> ui_out_emit_tuple;
+typedef ui_out_emit_type<ui_out_type_list> ui_out_emit_list;
+
 #endif /* UI_OUT_H */
-- 
2.9.3

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

* [RFA 5/5] Remove some gotos from Python
  2017-01-15 13:43 [RFA 0/5] more cleanup removal in Python Tom Tromey
                   ` (3 preceding siblings ...)
  2017-01-15 13:43 ` [RFA 2/5] Introduce ui_file_up and use it to remove cleanups Tom Tromey
@ 2017-01-15 13:43 ` Tom Tromey
  2017-02-09 13:03   ` Pedro Alves
  4 siblings, 1 reply; 30+ messages in thread
From: Tom Tromey @ 2017-01-15 13:43 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This patch slightly refactors a couple of spots in the Python code to
avoid some gotos.

2017-01-15  Tom Tromey  <tom@tromey.com>

	* python/python.c (do_start_initialization): New function, from
	_initialize_python.
	(_initialize_python): Call do_start_initialization.
	* python/py-linetable.c (ltpy_iternext): Use explicit returns, not
	goto.
---
 gdb/ChangeLog             |   8 ++
 gdb/python/py-linetable.c |  14 ++--
 gdb/python/python.c       | 192 ++++++++++++++++++++++++----------------------
 3 files changed, 115 insertions(+), 99 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index c32192d..7eaeb6c 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,13 @@
 2017-01-15  Tom Tromey  <tom@tromey.com>
 
+	* python/python.c (do_start_initialization): New function, from
+	_initialize_python.
+	(_initialize_python): Call do_start_initialization.
+	* python/py-linetable.c (ltpy_iternext): Use explicit returns, not
+	goto.
+
+2017-01-15  Tom Tromey  <tom@tromey.com>
+
 	* python/py-prettyprint.c (pretty_print_one_value): Use
 	gdbpy_ref.
 
diff --git a/gdb/python/py-linetable.c b/gdb/python/py-linetable.c
index 1f73ff7..5fe352d 100644
--- a/gdb/python/py-linetable.c
+++ b/gdb/python/py-linetable.c
@@ -407,7 +407,10 @@ ltpy_iternext (PyObject *self)
   LTPY_REQUIRE_VALID (iter_obj->source, symtab);
 
   if (iter_obj->current_index >= SYMTAB_LINETABLE (symtab)->nitems)
-    goto stop_iteration;
+    {
+      PyErr_SetNone (PyExc_StopIteration);
+      return NULL;
+    }
 
   item = &(SYMTAB_LINETABLE (symtab)->item[iter_obj->current_index]);
 
@@ -419,7 +422,10 @@ ltpy_iternext (PyObject *self)
 
       /* Exit if the internal value is the last item in the line table.  */
       if (iter_obj->current_index >= SYMTAB_LINETABLE (symtab)->nitems)
-	goto stop_iteration;
+	{
+	  PyErr_SetNone (PyExc_StopIteration);
+	  return NULL;
+	}
       item = &(SYMTAB_LINETABLE (symtab)->item[iter_obj->current_index]);
     }
 
@@ -427,10 +433,6 @@ ltpy_iternext (PyObject *self)
   iter_obj->current_index++;
 
   return obj;
-
- stop_iteration:
-  PyErr_SetNone (PyExc_StopIteration);
-  return NULL;
 }
 
 /* Implementation of gdb.LineTableIterator.is_valid (self) -> Boolean.
diff --git a/gdb/python/python.c b/gdb/python/python.c
index ab5a6a4..69851c3 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -1524,8 +1524,10 @@ finalize_python (void *ignore)
 /* Provide a prototype to silence -Wmissing-prototypes.  */
 extern initialize_file_ftype _initialize_python;
 
-void
-_initialize_python (void)
+#ifdef HAVE_PYTHON
+
+static bool
+do_start_initialization ()
 {
   char *progname;
 #ifdef IS_PY3K
@@ -1535,77 +1537,6 @@ _initialize_python (void)
   wchar_t *progname_copy;
 #endif
 
-  add_com ("python-interactive", class_obscure,
-	   python_interactive_command,
-#ifdef HAVE_PYTHON
-	   _("\
-Start an interactive Python prompt.\n\
-\n\
-To return to GDB, type the EOF character (e.g., Ctrl-D on an empty\n\
-prompt).\n\
-\n\
-Alternatively, a single-line Python command can be given as an\n\
-argument, and if the command is an expression, the result will be\n\
-printed.  For example:\n\
-\n\
-    (gdb) python-interactive 2 + 3\n\
-    5\n\
-")
-#else /* HAVE_PYTHON */
-	   _("\
-Start a Python interactive prompt.\n\
-\n\
-Python scripting is not supported in this copy of GDB.\n\
-This command is only a placeholder.")
-#endif /* HAVE_PYTHON */
-	   );
-  add_com_alias ("pi", "python-interactive", class_obscure, 1);
-
-  add_com ("python", class_obscure, python_command,
-#ifdef HAVE_PYTHON
-	   _("\
-Evaluate a Python command.\n\
-\n\
-The command can be given as an argument, for instance:\n\
-\n\
-    python print 23\n\
-\n\
-If no argument is given, the following lines are read and used\n\
-as the Python commands.  Type a line containing \"end\" to indicate\n\
-the end of the command.")
-#else /* HAVE_PYTHON */
-	   _("\
-Evaluate a Python command.\n\
-\n\
-Python scripting is not supported in this copy of GDB.\n\
-This command is only a placeholder.")
-#endif /* HAVE_PYTHON */
-	   );
-  add_com_alias ("py", "python", class_obscure, 1);
-
-  /* Add set/show python print-stack.  */
-  add_prefix_cmd ("python", no_class, user_show_python,
-		  _("Prefix command for python preference settings."),
-		  &user_show_python_list, "show python ", 0,
-		  &showlist);
-
-  add_prefix_cmd ("python", no_class, user_set_python,
-		  _("Prefix command for python preference settings."),
-		  &user_set_python_list, "set python ", 0,
-		  &setlist);
-
-  add_setshow_enum_cmd ("print-stack", no_class, python_excp_enums,
-			&gdbpy_should_print_stack, _("\
-Set mode for Python stack dump on error."), _("\
-Show the mode of Python stack printing on error."), _("\
-none  == no stack or message will be printed.\n\
-full == a message and a stack will be printed.\n\
-message == an error message without a stack will be printed."),
-			NULL, NULL,
-			&user_set_python_list,
-			&user_show_python_list);
-
-#ifdef HAVE_PYTHON
 #ifdef WITH_PYTHON_PATH
   /* Work around problem where python gets confused about where it is,
      and then can't find its libraries, etc.
@@ -1624,14 +1555,14 @@ message == an error message without a stack will be printed."),
     {
       xfree (oldloc);
       fprintf (stderr, "out of memory\n");
-      return;
+      return false;
     }
   count = mbstowcs (progname_copy, progname, progsize + 1);
   if (count == (size_t) -1)
     {
       xfree (oldloc);
       fprintf (stderr, "Could not convert python path to string\n");
-      return;
+      return false;
     }
   setlocale (LC_ALL, oldloc);
   xfree (oldloc);
@@ -1656,7 +1587,7 @@ message == an error message without a stack will be printed."),
   gdb_module = Py_InitModule ("_gdb", python_GdbMethods);
 #endif
   if (gdb_module == NULL)
-    goto fail;
+    return false;
 
   /* The casts to (char*) are for python 2.4.  */
   if (PyModule_AddStringConstant (gdb_module, "VERSION", (char*) version) < 0
@@ -1664,31 +1595,31 @@ message == an error message without a stack will be printed."),
 				     (char*) host_name) < 0
       || PyModule_AddStringConstant (gdb_module, "TARGET_CONFIG",
 				     (char*) target_name) < 0)
-    goto fail;
+    return false;
 
   /* Add stream constants.  */
   if (PyModule_AddIntConstant (gdb_module, "STDOUT", 0) < 0
       || PyModule_AddIntConstant (gdb_module, "STDERR", 1) < 0
       || PyModule_AddIntConstant (gdb_module, "STDLOG", 2) < 0)
-    goto fail;
+    return false;
 
   gdbpy_gdb_error = PyErr_NewException ("gdb.error", PyExc_RuntimeError, NULL);
   if (gdbpy_gdb_error == NULL
       || gdb_pymodule_addobject (gdb_module, "error", gdbpy_gdb_error) < 0)
-    goto fail;
+    return false;
 
   gdbpy_gdb_memory_error = PyErr_NewException ("gdb.MemoryError",
 					       gdbpy_gdb_error, NULL);
   if (gdbpy_gdb_memory_error == NULL
       || gdb_pymodule_addobject (gdb_module, "MemoryError",
 				 gdbpy_gdb_memory_error) < 0)
-    goto fail;
+    return false;
 
   gdbpy_gdberror_exc = PyErr_NewException ("gdb.GdbError", NULL, NULL);
   if (gdbpy_gdberror_exc == NULL
       || gdb_pymodule_addobject (gdb_module, "GdbError",
 				 gdbpy_gdberror_exc) < 0)
-    goto fail;
+    return false;
 
   gdbpy_initialize_gdb_readline ();
 
@@ -1729,26 +1660,26 @@ message == an error message without a stack will be printed."),
       || gdbpy_initialize_arch () < 0
       || gdbpy_initialize_xmethods () < 0
       || gdbpy_initialize_unwind () < 0)
-    goto fail;
+    return false;
 
   gdbpy_to_string_cst = PyString_FromString ("to_string");
   if (gdbpy_to_string_cst == NULL)
-    goto fail;
+    return false;
   gdbpy_children_cst = PyString_FromString ("children");
   if (gdbpy_children_cst == NULL)
-    goto fail;
+    return false;
   gdbpy_display_hint_cst = PyString_FromString ("display_hint");
   if (gdbpy_display_hint_cst == NULL)
-    goto fail;
+    return false;
   gdbpy_doc_cst = PyString_FromString ("__doc__");
   if (gdbpy_doc_cst == NULL)
-    goto fail;
+    return false;
   gdbpy_enabled_cst = PyString_FromString ("enabled");
   if (gdbpy_enabled_cst == NULL)
-    goto fail;
+    return false;
   gdbpy_value_cst = PyString_FromString ("value");
   if (gdbpy_value_cst == NULL)
-    goto fail;
+    return false;
 
   /* Release the GIL while gdb runs.  */
   PyThreadState_Swap (NULL);
@@ -1756,14 +1687,89 @@ message == an error message without a stack will be printed."),
 
   make_final_cleanup (finalize_python, NULL);
 
+  /* Only set this when initialization has succeeded.  */
   gdb_python_initialized = 1;
-  return;
+  return true;
+}
+
+#endif /* HAVE_PYTHON */
+
+void
+_initialize_python (void)
+{
+  add_com ("python-interactive", class_obscure,
+	   python_interactive_command,
+#ifdef HAVE_PYTHON
+	   _("\
+Start an interactive Python prompt.\n\
+\n\
+To return to GDB, type the EOF character (e.g., Ctrl-D on an empty\n\
+prompt).\n\
+\n\
+Alternatively, a single-line Python command can be given as an\n\
+argument, and if the command is an expression, the result will be\n\
+printed.  For example:\n\
+\n\
+    (gdb) python-interactive 2 + 3\n\
+    5\n\
+")
+#else /* HAVE_PYTHON */
+	   _("\
+Start a Python interactive prompt.\n\
+\n\
+Python scripting is not supported in this copy of GDB.\n\
+This command is only a placeholder.")
+#endif /* HAVE_PYTHON */
+	   );
+  add_com_alias ("pi", "python-interactive", class_obscure, 1);
+
+  add_com ("python", class_obscure, python_command,
+#ifdef HAVE_PYTHON
+	   _("\
+Evaluate a Python command.\n\
+\n\
+The command can be given as an argument, for instance:\n\
+\n\
+    python print 23\n\
+\n\
+If no argument is given, the following lines are read and used\n\
+as the Python commands.  Type a line containing \"end\" to indicate\n\
+the end of the command.")
+#else /* HAVE_PYTHON */
+	   _("\
+Evaluate a Python command.\n\
+\n\
+Python scripting is not supported in this copy of GDB.\n\
+This command is only a placeholder.")
+#endif /* HAVE_PYTHON */
+	   );
+  add_com_alias ("py", "python", class_obscure, 1);
+
+  /* Add set/show python print-stack.  */
+  add_prefix_cmd ("python", no_class, user_show_python,
+		  _("Prefix command for python preference settings."),
+		  &user_show_python_list, "show python ", 0,
+		  &showlist);
 
- fail:
-  gdbpy_print_stack ();
-  /* Do not set 'gdb_python_initialized'.  */
-  return;
+  add_prefix_cmd ("python", no_class, user_set_python,
+		  _("Prefix command for python preference settings."),
+		  &user_set_python_list, "set python ", 0,
+		  &setlist);
 
+  add_setshow_enum_cmd ("print-stack", no_class, python_excp_enums,
+			&gdbpy_should_print_stack, _("\
+Set mode for Python stack dump on error."), _("\
+Show the mode of Python stack printing on error."), _("\
+none  == no stack or message will be printed.\n\
+full == a message and a stack will be printed.\n\
+message == an error message without a stack will be printed."),
+			NULL, NULL,
+			&user_set_python_list,
+			&user_show_python_list);
+
+#ifdef HAVE_PYTHON
+  if (!do_start_initialization () && PyErr_Occurred ())
+    gdbpy_print_stack ();
 #endif /* HAVE_PYTHON */
 }
 
-- 
2.9.3

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

* [RFA 0/5] more cleanup removal in Python
@ 2017-01-15 13:43 Tom Tromey
  2017-01-15 13:43 ` [RFA 3/5] Introduce gdbpy_subclass and use it to simplify some logic Tom Tromey
                   ` (4 more replies)
  0 siblings, 5 replies; 30+ messages in thread
From: Tom Tromey @ 2017-01-15 13:43 UTC (permalink / raw)
  To: gdb-patches

This is the next installment in my ongoing quest to remove cleanups,
manual refcount management, and "goto" from the Python layer.  I think
that, on the whole, these changes make the code simpler to read and
write.

I tested this via the buildbot.

For the most part I think the changes are self-explanatory.  There is
a renaming opportunity in patch #3, so perhaps one more round will be
needed for this series.

Tom

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

* [RFA 4/5] Change one more spot to use gdbpy_ref
  2017-01-15 13:43 [RFA 0/5] more cleanup removal in Python Tom Tromey
  2017-01-15 13:43 ` [RFA 3/5] Introduce gdbpy_subclass and use it to simplify some logic Tom Tromey
@ 2017-01-15 13:43 ` Tom Tromey
  2017-02-09 12:52   ` Pedro Alves
  2017-01-15 13:43 ` [RFA 1/5] Remove some ui_out-related cleanups from Python Tom Tromey
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 30+ messages in thread
From: Tom Tromey @ 2017-01-15 13:43 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This patch changes one more spot in the Python layer to use gdbpy_ref.

2017-01-15  Tom Tromey  <tom@tromey.com>

	* python/py-prettyprint.c (pretty_print_one_value): Use
	gdbpy_ref.
---
 gdb/ChangeLog               |  5 +++++
 gdb/python/py-prettyprint.c | 15 ++++++++-------
 2 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 61e4ca4..c32192d 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,10 @@
 2017-01-15  Tom Tromey  <tom@tromey.com>
 
+	* python/py-prettyprint.c (pretty_print_one_value): Use
+	gdbpy_ref.
+
+2017-01-15  Tom Tromey  <tom@tromey.com>
+
 	* python/py-cmd.c (cmdpy_destroyer): Use gdbpy_subclass.
 	* python/py-breakpoint.c (gdbpy_breakpoint_deleted): Use
 	gdbpy_subclass.
diff --git a/gdb/python/py-prettyprint.c b/gdb/python/py-prettyprint.c
index d6d157a..6132201 100644
--- a/gdb/python/py-prettyprint.c
+++ b/gdb/python/py-prettyprint.c
@@ -189,21 +189,22 @@ find_pretty_printer (PyObject *value)
 static PyObject *
 pretty_print_one_value (PyObject *printer, struct value **out_value)
 {
-  PyObject *result = NULL;
+  gdbpy_ref result;
 
   *out_value = NULL;
   TRY
     {
-      result = PyObject_CallMethodObjArgs (printer, gdbpy_to_string_cst, NULL);
-      if (result)
+      result.reset (PyObject_CallMethodObjArgs (printer, gdbpy_to_string_cst,
+						NULL));
+      if (result != NULL)
 	{
-	  if (! gdbpy_is_string (result) && ! gdbpy_is_lazy_string (result)
+	  if (! gdbpy_is_string (result.get ())
+	      && ! gdbpy_is_lazy_string (result.get ())
 	      && result != Py_None)
 	    {
-	      *out_value = convert_value_from_python (result);
+	      *out_value = convert_value_from_python (result.get ());
 	      if (PyErr_Occurred ())
 		*out_value = NULL;
-	      Py_DECREF (result);
 	      result = NULL;
 	    }
 	}
@@ -213,7 +214,7 @@ pretty_print_one_value (PyObject *printer, struct value **out_value)
     }
   END_CATCH
 
-  return result;
+  return result.release ();
 }
 
 /* Return the display hint for the object printer, PRINTER.  Return
-- 
2.9.3

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

* [RFA 3/5] Introduce gdbpy_subclass and use it to simplify some logic
  2017-01-15 13:43 [RFA 0/5] more cleanup removal in Python Tom Tromey
@ 2017-01-15 13:43 ` Tom Tromey
  2017-01-24 20:21   ` Simon Marchi
  2017-02-09 13:00   ` Pedro Alves
  2017-01-15 13:43 ` [RFA 4/5] Change one more spot to use gdbpy_ref Tom Tromey
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 30+ messages in thread
From: Tom Tromey @ 2017-01-15 13:43 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This introduces a new typed variant of gdbpy_ref, called
gdbpy_subclass.  This is then used to simplify logic in various parts
of the Python layer; for example removing repeated error code or
removing gotos.

I wouldn't mind a better than than "gdb_subclass".  One idea was to
use gdb_ref with a default template parameter, and then change the
existing uses of "gdb_ref" to "gdb_ref<>".

2017-01-15  Tom Tromey  <tom@tromey.com>

	* python/py-cmd.c (cmdpy_destroyer): Use gdbpy_subclass.
	* python/py-breakpoint.c (gdbpy_breakpoint_deleted): Use
	gdbpy_subclass.
	* python/py-type.c (field_new): Use gdbpy_subclass.
	* python/py-symtab.c (symtab_and_line_to_sal_object): Use
	gdbpy_subclass.
	* python/py-progspace.c (pspy_new): Use gdbpy_subclass.
	(py_free_pspace): Likewise.
	(pspace_to_pspace_object): Likewise.
	* python/py-objfile.c (objfpy_new): Use gdbpy_subclass.
	(py_free_objfile): Likewise.
	(objfile_to_objfile_object): Likewise.
	* python/py-inferior.c (delete_thread_object): Use
	gdbpy_subclass.
	(infpy_read_memory): Likewise.
	(py_free_inferior): Likewise.
	* python/py-evtregistry.c (create_eventregistry_object): Use
	gdbpy_subclass.
	* python/py-event.c (create_event_object): Use gdbpy_subclass.
	* python/py-ref.h (gdbpy_ref_policy): Now a template class.
	(gdbpy_subclass): New typedef.
---
 gdb/ChangeLog               | 24 ++++++++++++++++++++++++
 gdb/python/py-breakpoint.c  |  8 +++-----
 gdb/python/py-cmd.c         |  5 +----
 gdb/python/py-event.c       | 16 +++++-----------
 gdb/python/py-evtregistry.c | 17 +++++++----------
 gdb/python/py-frame.c       |  9 ++++-----
 gdb/python/py-inferior.c    | 26 +++++++++-----------------
 gdb/python/py-objfile.c     | 41 +++++++++++++++++------------------------
 gdb/python/py-progspace.c   | 40 +++++++++++++++++-----------------------
 gdb/python/py-ref.h         | 14 ++++++++++----
 gdb/python/py-symtab.c      | 17 +++++++----------
 gdb/python/py-type.c        | 12 +++++-------
 12 files changed, 109 insertions(+), 120 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 08670fb..61e4ca4 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,29 @@
 2017-01-15  Tom Tromey  <tom@tromey.com>
 
+	* python/py-cmd.c (cmdpy_destroyer): Use gdbpy_subclass.
+	* python/py-breakpoint.c (gdbpy_breakpoint_deleted): Use
+	gdbpy_subclass.
+	* python/py-type.c (field_new): Use gdbpy_subclass.
+	* python/py-symtab.c (symtab_and_line_to_sal_object): Use
+	gdbpy_subclass.
+	* python/py-progspace.c (pspy_new): Use gdbpy_subclass.
+	(py_free_pspace): Likewise.
+	(pspace_to_pspace_object): Likewise.
+	* python/py-objfile.c (objfpy_new): Use gdbpy_subclass.
+	(py_free_objfile): Likewise.
+	(objfile_to_objfile_object): Likewise.
+	* python/py-inferior.c (delete_thread_object): Use
+	gdbpy_subclass.
+	(infpy_read_memory): Likewise.
+	(py_free_inferior): Likewise.
+	* python/py-evtregistry.c (create_eventregistry_object): Use
+	gdbpy_subclass.
+	* python/py-event.c (create_event_object): Use gdbpy_subclass.
+	* python/py-ref.h (gdbpy_ref_policy): Now a template class.
+	(gdbpy_subclass): New typedef.
+
+2017-01-15  Tom Tromey  <tom@tromey.com>
+
 	* ui-file.h (mem_fileopen): Return ui_file_up.
 	(ui_file_up): New typedef.
 	(ui_file_deleter): New struct.
diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c
index f086725..f95a396 100644
--- a/gdb/python/py-breakpoint.c
+++ b/gdb/python/py-breakpoint.c
@@ -913,25 +913,23 @@ gdbpy_breakpoint_deleted (struct breakpoint *b)
   int num = b->number;
   PyGILState_STATE state;
   struct breakpoint *bp = NULL;
-  gdbpy_breakpoint_object *bp_obj;
 
   state = PyGILState_Ensure ();
   bp = get_breakpoint (num);
   if (bp)
     {
-      bp_obj = bp->py_bp_object;
-      if (bp_obj)
+      gdbpy_subclass<gdbpy_breakpoint_object> bp_obj (bp->py_bp_object);
+      if (bp_obj != NULL)
 	{
 	  if (!evregpy_no_listeners_p (gdb_py_events.breakpoint_deleted))
 	    {
-	      if (evpy_emit_event ((PyObject *) bp_obj,
+	      if (evpy_emit_event ((PyObject *) bp_obj.get (),
 				   gdb_py_events.breakpoint_deleted) < 0)
 		gdbpy_print_stack ();
 	    }
 
 	  bp_obj->bp = NULL;
 	  --bppy_live;
-	  Py_DECREF (bp_obj);
 	}
     }
   PyGILState_Release (state);
diff --git a/gdb/python/py-cmd.c b/gdb/python/py-cmd.c
index 28c391c..b798c8a 100644
--- a/gdb/python/py-cmd.c
+++ b/gdb/python/py-cmd.c
@@ -95,14 +95,11 @@ cmdpy_dont_repeat (PyObject *self, PyObject *args)
 static void
 cmdpy_destroyer (struct cmd_list_element *self, void *context)
 {
-  cmdpy_object *cmd;
-
   gdbpy_enter enter_py (get_current_arch (), current_language);
 
   /* Release our hold on the command object.  */
-  cmd = (cmdpy_object *) context;
+  gdbpy_subclass<cmdpy_object> cmd ((cmdpy_object *) context);
   cmd->command = NULL;
-  Py_DECREF (cmd);
 
   /* We allocated the name, doc string, and perhaps the prefix
      name.  */
diff --git a/gdb/python/py-event.c b/gdb/python/py-event.c
index a50da1f..1369fb4 100644
--- a/gdb/python/py-event.c
+++ b/gdb/python/py-event.c
@@ -30,21 +30,15 @@ evpy_dealloc (PyObject *self)
 PyObject *
 create_event_object (PyTypeObject *py_type)
 {
-  event_object *event_obj;
-
-  event_obj = PyObject_New (event_object, py_type);
-  if (!event_obj)
-    goto fail;
+  gdbpy_subclass<event_object> event_obj (PyObject_New (event_object, py_type));
+  if (event_obj == NULL)
+    return NULL;
 
   event_obj->dict = PyDict_New ();
   if (!event_obj->dict)
-    goto fail;
-
-  return (PyObject*) event_obj;
+    return NULL;
 
- fail:
-  Py_XDECREF (event_obj);
-  return NULL;
+  return (PyObject*) event_obj.release ();
 }
 
 /* Add the attribute ATTR to the event object EVENT.  In
diff --git a/gdb/python/py-evtregistry.c b/gdb/python/py-evtregistry.c
index 81bf927..023a7ce 100644
--- a/gdb/python/py-evtregistry.c
+++ b/gdb/python/py-evtregistry.c
@@ -20,6 +20,7 @@
 #include "defs.h"
 #include "command.h"
 #include "py-events.h"
+#include "py-ref.h"
 
 events_object gdb_py_events;
 
@@ -79,22 +80,18 @@ evregpy_disconnect (PyObject *self, PyObject *function)
 eventregistry_object *
 create_eventregistry_object (void)
 {
-  eventregistry_object *eventregistry_obj;
+  gdbpy_subclass<eventregistry_object>
+    eventregistry_obj (PyObject_New (eventregistry_object,
+				     &eventregistry_object_type));
 
-  eventregistry_obj = PyObject_New (eventregistry_object,
-                                    &eventregistry_object_type);
-
-  if (!eventregistry_obj)
+  if (eventregistry_obj == NULL)
     return NULL;
 
   eventregistry_obj->callbacks = PyList_New (0);
   if (!eventregistry_obj->callbacks)
-    {
-      Py_DECREF (eventregistry_obj);
-      return NULL;
-    }
+    return NULL;
 
-  return eventregistry_obj;
+  return eventregistry_obj.release ();
 }
 
 static void
diff --git a/gdb/python/py-frame.c b/gdb/python/py-frame.c
index 3594be2..8b07f72 100644
--- a/gdb/python/py-frame.c
+++ b/gdb/python/py-frame.c
@@ -28,6 +28,7 @@
 #include "symfile.h"
 #include "objfiles.h"
 #include "user-regs.h"
+#include "py-ref.h"
 
 typedef struct {
   PyObject_HEAD
@@ -361,9 +362,8 @@ frapy_function (PyObject *self, PyObject *args)
 PyObject *
 frame_info_to_frame_object (struct frame_info *frame)
 {
-  frame_object *frame_obj;
-
-  frame_obj = PyObject_New (frame_object, &frame_object_type);
+  gdbpy_subclass<frame_object> frame_obj
+    (PyObject_New (frame_object, &frame_object_type));
   if (frame_obj == NULL)
     return NULL;
 
@@ -389,13 +389,12 @@ frame_info_to_frame_object (struct frame_info *frame)
     }
   CATCH (except, RETURN_MASK_ALL)
     {
-      Py_DECREF (frame_obj);
       gdbpy_convert_exception (except);
       return NULL;
     }
   END_CATCH
 
-  return (PyObject *) frame_obj;
+  return (PyObject *) frame_obj.release ();
 }
 
 /* Implementation of gdb.Frame.older (self) -> gdb.Frame.
diff --git a/gdb/python/py-inferior.c b/gdb/python/py-inferior.c
index 9e10d62..e2f21d4 100644
--- a/gdb/python/py-inferior.c
+++ b/gdb/python/py-inferior.c
@@ -307,7 +307,6 @@ add_thread_object (struct thread_info *tp)
 static void
 delete_thread_object (struct thread_info *tp, int ignore)
 {
-  inferior_object *inf_obj;
   struct threadlist_entry **entry, *tmp;
 
   if (!gdb_python_initialized)
@@ -315,9 +314,9 @@ delete_thread_object (struct thread_info *tp, int ignore)
 
   gdbpy_enter enter_py (python_gdbarch, python_language);
 
-  inf_obj
-    = (inferior_object *) find_inferior_object (ptid_get_pid (tp->ptid));
-  if (!inf_obj)
+  gdbpy_subclass<inferior_object> inf_obj
+    ((inferior_object *) find_inferior_object (ptid_get_pid (tp->ptid)));
+  if (inf_obj == NULL)
     return;
 
   /* Find thread entry in its inferior's thread_list.  */
@@ -327,10 +326,7 @@ delete_thread_object (struct thread_info *tp, int ignore)
       break;
 
   if (!*entry)
-    {
-      Py_DECREF (inf_obj);
-      return;
-    }
+    return;
 
   tmp = *entry;
   tmp->thread_obj->thread = NULL;
@@ -339,7 +335,6 @@ delete_thread_object (struct thread_info *tp, int ignore)
   inf_obj->nthreads--;
 
   Py_DECREF (tmp->thread_obj);
-  Py_DECREF (inf_obj);
   xfree (tmp);
 }
 
@@ -446,7 +441,6 @@ infpy_read_memory (PyObject *self, PyObject *args, PyObject *kw)
 {
   CORE_ADDR addr, length;
   gdb_byte *buffer = NULL;
-  membuf_object *membuf_obj;
   PyObject *addr_obj, *length_obj, *result;
   static char *keywords[] = { "address", "length", NULL };
 
@@ -471,7 +465,8 @@ infpy_read_memory (PyObject *self, PyObject *args, PyObject *kw)
     }
   END_CATCH
 
-  membuf_obj = PyObject_New (membuf_object, &membuf_object_type);
+  gdbpy_subclass<membuf_object> membuf_obj (PyObject_New (membuf_object,
+							  &membuf_object_type));
   if (membuf_obj == NULL)
     {
       xfree (buffer);
@@ -483,12 +478,11 @@ infpy_read_memory (PyObject *self, PyObject *args, PyObject *kw)
   membuf_obj->length = length;
 
 #ifdef IS_PY3K
-  result = PyMemoryView_FromObject ((PyObject *) membuf_obj);
+  result = PyMemoryView_FromObject ((PyObject *) membuf_obj.get ());
 #else
-  result = PyBuffer_FromReadWriteObject ((PyObject *) membuf_obj, 0,
+  result = PyBuffer_FromReadWriteObject ((PyObject *) membuf_obj.get (), 0,
 					 Py_END_OF_BUFFER);
 #endif
-  Py_DECREF (membuf_obj);
 
   return result;
 }
@@ -776,7 +770,7 @@ infpy_dealloc (PyObject *obj)
 static void
 py_free_inferior (struct inferior *inf, void *datum)
 {
-  inferior_object *inf_obj = (inferior_object *) datum;
+  gdbpy_subclass<inferior_object> inf_obj ((inferior_object *) datum);
   struct threadlist_entry *th_entry, *th_tmp;
 
   if (!gdb_python_initialized)
@@ -797,8 +791,6 @@ py_free_inferior (struct inferior *inf, void *datum)
     }
 
   inf_obj->nthreads = 0;
-
-  Py_DECREF ((PyObject *) inf_obj);
 }
 
 /* Implementation of gdb.selected_inferior() -> gdb.Inferior.
diff --git a/gdb/python/py-objfile.c b/gdb/python/py-objfile.c
index 694fa09..136dfa7 100644
--- a/gdb/python/py-objfile.c
+++ b/gdb/python/py-objfile.c
@@ -24,6 +24,7 @@
 #include "language.h"
 #include "build-id.h"
 #include "symtab.h"
+#include "py-ref.h"
 
 typedef struct
 {
@@ -227,18 +228,16 @@ objfpy_initialize (objfile_object *self)
 static PyObject *
 objfpy_new (PyTypeObject *type, PyObject *args, PyObject *keywords)
 {
-  objfile_object *self = (objfile_object *) type->tp_alloc (type, 0);
+  gdbpy_subclass<objfile_object> self
+    ((objfile_object *) type->tp_alloc (type, 0));
 
-  if (self)
+  if (self != NULL)
     {
-      if (!objfpy_initialize (self))
-	{
-	  Py_DECREF (self);
-	  return NULL;
-	}
+      if (!objfpy_initialize (self.get ()))
+	return NULL;
     }
 
-  return (PyObject *) self;
+  return (PyObject *) self.release ();
 }
 
 PyObject *
@@ -612,11 +611,9 @@ gdbpy_lookup_objfile (PyObject *self, PyObject *args, PyObject *kw)
 static void
 py_free_objfile (struct objfile *objfile, void *datum)
 {
-  objfile_object *object = (objfile_object *) datum;
-
   gdbpy_enter enter_py (get_objfile_arch (objfile), current_language);
+  gdbpy_subclass<objfile_object> object ((objfile_object *) datum);
   object->objfile = NULL;
-  Py_DECREF ((PyObject *) object);
 }
 
 /* Return a borrowed reference to the Python object of type Objfile
@@ -627,26 +624,22 @@ py_free_objfile (struct objfile *objfile, void *datum)
 PyObject *
 objfile_to_objfile_object (struct objfile *objfile)
 {
-  objfile_object *object;
-
-  object = (objfile_object *) objfile_data (objfile, objfpy_objfile_data_key);
-  if (!object)
+  gdbpy_subclass<objfile_object> object
+    ((objfile_object *) objfile_data (objfile, objfpy_objfile_data_key));
+  if (object == NULL)
     {
-      object = PyObject_New (objfile_object, &objfile_object_type);
-      if (object)
+      object.reset (PyObject_New (objfile_object, &objfile_object_type));
+      if (object != NULL)
 	{
-	  if (!objfpy_initialize (object))
-	    {
-	      Py_DECREF (object);
-	      return NULL;
-	    }
+	  if (!objfpy_initialize (object.get ()))
+	    return NULL;
 
 	  object->objfile = objfile;
-	  set_objfile_data (objfile, objfpy_objfile_data_key, object);
+	  set_objfile_data (objfile, objfpy_objfile_data_key, object.get ());
 	}
     }
 
-  return (PyObject *) object;
+  return (PyObject *) object.release ();
 }
 
 int
diff --git a/gdb/python/py-progspace.c b/gdb/python/py-progspace.c
index b0d9458..70e052b 100644
--- a/gdb/python/py-progspace.c
+++ b/gdb/python/py-progspace.c
@@ -24,6 +24,7 @@
 #include "objfiles.h"
 #include "language.h"
 #include "arch-utils.h"
+#include "py-ref.h"
 
 typedef struct
 {
@@ -128,18 +129,16 @@ pspy_initialize (pspace_object *self)
 static PyObject *
 pspy_new (PyTypeObject *type, PyObject *args, PyObject *keywords)
 {
-  pspace_object *self = (pspace_object *) type->tp_alloc (type, 0);
+  gdbpy_subclass<pspace_object> self
+    ((pspace_object *) type->tp_alloc (type, 0));
 
-  if (self)
+  if (self != NULL)
     {
-      if (!pspy_initialize (self))
-	{
-	  Py_DECREF (self);
-	  return NULL;
-	}
+      if (!pspy_initialize (self.get ()))
+	return NULL;
     }
 
-  return (PyObject *) self;
+  return (PyObject *) self.release ();
 }
 
 PyObject *
@@ -323,7 +322,7 @@ pspy_set_type_printers (PyObject *o, PyObject *value, void *ignore)
 static void
 py_free_pspace (struct program_space *pspace, void *datum)
 {
-  pspace_object *object = (pspace_object *) datum;
+  gdbpy_subclass<pspace_object> object ((pspace_object *) datum);
   /* This is a fiction, but we're in a nasty spot: The pspace is in the
      process of being deleted, we can't rely on anything in it.  Plus
      this is one time when the current program space and current inferior
@@ -337,7 +336,6 @@ py_free_pspace (struct program_space *pspace, void *datum)
 
   gdbpy_enter enter_py (arch, current_language);
   object->pspace = NULL;
-  Py_DECREF ((PyObject *) object);
 }
 
 /* Return a borrowed reference to the Python object of type Pspace
@@ -348,26 +346,22 @@ py_free_pspace (struct program_space *pspace, void *datum)
 PyObject *
 pspace_to_pspace_object (struct program_space *pspace)
 {
-  pspace_object *object;
-
-  object = (pspace_object *) program_space_data (pspace, pspy_pspace_data_key);
-  if (!object)
+  gdbpy_subclass<pspace_object> object
+    ((pspace_object *) program_space_data (pspace, pspy_pspace_data_key));
+  if (object == NULL)
     {
-      object = PyObject_New (pspace_object, &pspace_object_type);
-      if (object)
+      object.reset (PyObject_New (pspace_object, &pspace_object_type));
+      if (object != NULL)
 	{
-	  if (!pspy_initialize (object))
-	    {
-	      Py_DECREF (object);
-	      return NULL;
-	    }
+	  if (!pspy_initialize (object.get ()))
+	    return NULL;
 
 	  object->pspace = pspace;
-	  set_program_space_data (pspace, pspy_pspace_data_key, object);
+	  set_program_space_data (pspace, pspy_pspace_data_key, object.get ());
 	}
     }
 
-  return (PyObject *) object;
+  return (PyObject *) object.release ();
 }
 
 int
diff --git a/gdb/python/py-ref.h b/gdb/python/py-ref.h
index b2479bf..3430768 100644
--- a/gdb/python/py-ref.h
+++ b/gdb/python/py-ref.h
@@ -1,6 +1,6 @@
 /* Python reference-holding class
 
-   Copyright (C) 2016 Free Software Foundation, Inc.
+   Copyright (C) 2016, 2017 Free Software Foundation, Inc.
 
    This file is part of GDB.
 
@@ -23,20 +23,26 @@
 #include "common/gdb_ref_ptr.h"
 
 /* A policy class for gdb::ref_ptr for Python reference counting.  */
+template<typename T>
 struct gdbpy_ref_policy
 {
-  static void incref (PyObject *ptr)
+  static void incref (T *ptr)
   {
     Py_INCREF (ptr);
   }
 
-  static void decref (PyObject *ptr)
+  static void decref (T *ptr)
   {
     Py_DECREF (ptr);
   }
 };
 
 /* A gdb::ref_ptr that has been specialized for Python objects.  */
-typedef gdb::ref_ptr<PyObject, gdbpy_ref_policy> gdbpy_ref;
+typedef gdb::ref_ptr<PyObject, gdbpy_ref_policy<PyObject> > gdbpy_ref;
+
+/* A gdb::ref_ptr that has been specialized for "subclasses" of Python
+   objects.  */
+template<typename T> using gdbpy_subclass
+  = gdb::ref_ptr<T, gdbpy_ref_policy<T> >;
 
 #endif /* GDB_PYTHON_REF_H */
diff --git a/gdb/python/py-symtab.c b/gdb/python/py-symtab.c
index 9fdb42f..770c898 100644
--- a/gdb/python/py-symtab.c
+++ b/gdb/python/py-symtab.c
@@ -24,6 +24,7 @@
 #include "python-internal.h"
 #include "objfiles.h"
 #include "block.h"
+#include "py-ref.h"
 
 typedef struct stpy_symtab_object {
   PyObject_HEAD
@@ -436,19 +437,15 @@ symtab_to_symtab_object (struct symtab *symtab)
 PyObject *
 symtab_and_line_to_sal_object (struct symtab_and_line sal)
 {
-  sal_object *sal_obj;
-
-  sal_obj = PyObject_New (sal_object, &sal_object_type);
-  if (sal_obj)
+  gdbpy_subclass<sal_object> sal_obj (PyObject_New (sal_object,
+						    &sal_object_type));
+  if (sal_obj != NULL)
     {
-      if (set_sal (sal_obj, sal) < 0)
-	{
-	  Py_DECREF (sal_obj);
-	  return NULL;
-	}
+      if (set_sal (sal_obj.get (), sal) < 0)
+	return NULL;
     }
 
-  return (PyObject *) sal_obj;
+  return (PyObject *) sal_obj.release ();
 }
 
 /* Return struct symtab_and_line reference that is wrapped by this
diff --git a/gdb/python/py-type.c b/gdb/python/py-type.c
index 235e8b5..cc1bbf5 100644
--- a/gdb/python/py-type.c
+++ b/gdb/python/py-type.c
@@ -130,18 +130,16 @@ field_dealloc (PyObject *obj)
 static PyObject *
 field_new (void)
 {
-  field_object *result = PyObject_New (field_object, &field_object_type);
+  gdbpy_subclass<field_object> result (PyObject_New (field_object,
+						     &field_object_type));
 
-  if (result)
+  if (result != NULL)
     {
       result->dict = PyDict_New ();
       if (!result->dict)
-	{
-	  Py_DECREF (result);
-	  result = NULL;
-	}
+	return NULL;
     }
-  return (PyObject *) result;
+  return (PyObject *) result.release ();
 }
 
 \f
-- 
2.9.3

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

* Re: [RFA 1/5] Remove some ui_out-related cleanups from Python
  2017-01-15 13:43 ` [RFA 1/5] Remove some ui_out-related cleanups from Python Tom Tromey
@ 2017-01-15 21:52   ` Simon Marchi
  2017-01-16 16:13     ` Tom Tromey
  2017-01-16 11:19   ` Trevor Saunders
  1 sibling, 1 reply; 30+ messages in thread
From: Simon Marchi @ 2017-01-15 21:52 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

On 2017-01-15 08:42, Tom Tromey wrote:
> This patch introduces a bit of infrastructure -- namely, a minimal
> std::optional analogue called gdb::optional, and an RAII template
> class that works like make_cleanup_ui_out_tuple_begin_end or
> make_cleanup_ui_out_list_begin_end -- and then uses these in the
> Python code.  This removes a number of cleanups and generally
> simplifies this code.
> 
> std::optional is only available in C++17.  Normally I would have had
> this code check __cplusplus, but my gcc apparently isn't new enough to
> find <optional>, even with -std=c++1z; so, because I could not test
> it, the patch does not do this.
> 
> 2017-01-15  Tom Tromey  <tom@tromey.com>
> 
> 	* ui-out.h (ui_out_emit_type): New class.
> 	(ui_out_emit_tuple, ui_out_emit_list): New typedefs.
> 	* python/py-framefilter.c (py_print_single_arg): Use gdb::optional
> 	and ui_out_emit_tuple.
> 	(enumerate_locals): Likewise.
> 	(py_mi_print_variables, py_print_locals, py_print_args): Use
> 	ui_out_emit_list.
> 	(py_print_frame): Use gdb::optional, ui_out_emit_tuple,
> 	ui_out_emit_list.
> 	* common/gdb_option.h: New file.
> ---
>  gdb/ChangeLog               |  13 +++
>  gdb/common/gdb_option.h     |  87 ++++++++++++++++++

Any reason you did not name this gdb_optional.h?

Otherwise, it looks really nice.

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

* Re: [RFA 2/5] Introduce ui_file_up and use it to remove cleanups
  2017-01-15 13:43 ` [RFA 2/5] Introduce ui_file_up and use it to remove cleanups Tom Tromey
@ 2017-01-16  9:59   ` Trevor Saunders
  2017-01-16 17:58   ` Pedro Alves
  1 sibling, 0 replies; 30+ messages in thread
From: Trevor Saunders @ 2017-01-16  9:59 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

> +++ b/gdb/ada-lang.c
> @@ -7602,16 +7602,11 @@ ada_value_struct_elt (struct value *arg, char *name, int no_err)
>  static std::string
>  type_as_string (struct type *type)
>  {
> -  struct ui_file *tmp_stream = mem_fileopen ();
> -  struct cleanup *old_chain;
> -
> -  tmp_stream = mem_fileopen ();
> -  old_chain = make_cleanup_ui_file_delete (tmp_stream);
> +  ui_file_up tmp_stream = mem_fileopen ();
>  
> -  type_print (type, "", tmp_stream, -1);
> -  std::string str = ui_file_as_string (tmp_stream);
> +  type_print (type, "", tmp_stream.get (), -1);
> +  std::string str = ui_file_as_string (tmp_stream.get ());
>  
> -  do_cleanups (old_chain);
>    return str;

you could just get rid of the str variable here and a bunch of other
places right?  Though obviously that can happen later.

Trev

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

* Re: [RFA 1/5] Remove some ui_out-related cleanups from Python
  2017-01-15 13:43 ` [RFA 1/5] Remove some ui_out-related cleanups from Python Tom Tromey
  2017-01-15 21:52   ` Simon Marchi
@ 2017-01-16 11:19   ` Trevor Saunders
  2017-02-08 17:28     ` Pedro Alves
  1 sibling, 1 reply; 30+ messages in thread
From: Trevor Saunders @ 2017-01-16 11:19 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

> +++ b/gdb/common/gdb_option.h

might be nice to put it in include/ but fine to do that later when
something else actually wants it.

> +/* This class attempts to be a compatible subset of std::optional,
> +   which is slated to be available in C++17.  This class optionally
> +   holds an object of some type -- by default it is constructed not
> +   holding an object, but later the object can be "emplaced".  This is
> +   similar to using std::unique_ptr, but stack allocation is
> +   guaranteed.  */

 wording nit, but stack isn't quiet what you want there, I can imagine
 putting an optional<T> in some object that lives on the heap.

> +template<typename T>
> +class optional
> +{
> +public:
> +
> +  optional ()
> +    : m_instantiated (false)
> +  {
> +  }
> +
> +  ~optional ()
> +  {
> +    if (m_instantiated)
> +      destroy ();
> +  }
> +
> +  /* These aren't deleted in std::optional, but it was simpler to
> +     delete them here, because currently the users of this class don't
> +     need them, and making them depend on the definition of T is
> +     somewhat complicated.  */

I think you can make <type_traits> do most of it, but fair enough.

> +  /* True if the object was ever emplaced.  */
> +  bool m_instantiated;
> +
> +  /* The object.  */
> +  union
> +  {
> +    struct { } m_dummy;
> +    T m_item;
> +  };

It doesn't matter yet, but space wise it would be better to put the bool
last right? For example if sizeof(T) is 6, or if the optional is in some
larger structure with a bool next.

Trev

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

* Re: [RFA 1/5] Remove some ui_out-related cleanups from Python
  2017-01-15 21:52   ` Simon Marchi
@ 2017-01-16 16:13     ` Tom Tromey
  0 siblings, 0 replies; 30+ messages in thread
From: Tom Tromey @ 2017-01-16 16:13 UTC (permalink / raw)
  To: Simon Marchi; +Cc: Tom Tromey, gdb-patches

>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:

Simon> Any reason you did not name this gdb_optional.h?

No, no good reason.  I'll rename it.

Tom

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

* Re: [RFA 2/5] Introduce ui_file_up and use it to remove cleanups
  2017-01-15 13:43 ` [RFA 2/5] Introduce ui_file_up and use it to remove cleanups Tom Tromey
  2017-01-16  9:59   ` Trevor Saunders
@ 2017-01-16 17:58   ` Pedro Alves
  2017-01-16 19:08     ` Tom Tromey
  1 sibling, 1 reply; 30+ messages in thread
From: Pedro Alves @ 2017-01-16 17:58 UTC (permalink / raw)
  To: Tom Tromey, gdb-patches

On 01/15/2017 01:42 PM, Tom Tromey wrote:
> This introduces a new ui_file_up typedef, which is a specialization of
> unique_ptr that calls ui_file_delete.  This patch also changes
> mem_fileopen to return a ui_file_up, and fixes the users.  It also
> updates a few other spots in the Python code to use this rather than
> cleanups.
> 
> If at some point ui_file_delete is removed in favor of a destructor, I
> think the typedef could be changed and the default deletion policy
> used instead.

Urgh, guess you forgot/missed my 
palves/cxx-eliminate-cleanups branch ... :-(

The top commit does:

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Eliminate make_cleanup_ui_file_delete

And now that ui_file_as_string is in, let's eliminate it.  :-)

Makes ui_file a real C++ hierarchy.  mem_fileopen is replaced with a
new string_file class that is treated as a value class created on the
stack.  This alone eliminates most make_cleanup_ui_file_delete calls,
and, simplifies code a whole lot (diffstat shows almost 1k loc
dropped.)

string_file has a string() method that gives you a direct reference to
the internal std::string.  This is what replaces old (well, new)
ui_file_as_string, which used to alway return a new copy of the same
data the stream had inside..  With direct access via a writable
reference, we can instead move the string out of the string_stream.

Note I needed a tweak on scoped_restore.  That one should probably be
split out to a separate patch.

This exposed the need to make use of gnulib namespace support.
Otherwise, making use of read/write/printf/puts/etc symbol names will
clash on systems where gnulib replaces those functions, due to
'#define foo rpl_foo'.
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

I've been working on fixing that gnulib namespace issue since,
thinking that I'd fix it before posting that patch, but looks
like that was the wrong decision...  :-/  Simon's ui_out series
also had the same problem, but since common-defs.h includes
the system headers that might cause the problem before
any other header, it ends up not being such a bad problem
in practice...

Thanks,
Pedro Alves

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

* Re: [RFA 2/5] Introduce ui_file_up and use it to remove cleanups
  2017-01-16 17:58   ` Pedro Alves
@ 2017-01-16 19:08     ` Tom Tromey
  2017-01-17  1:40       ` Pedro Alves
  0 siblings, 1 reply; 30+ messages in thread
From: Tom Tromey @ 2017-01-16 19:08 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Tom Tromey, gdb-patches

>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> Urgh, guess you forgot/missed my 
Pedro> palves/cxx-eliminate-cleanups branch ... :-(

Yeah.  I knew of it but forgot what was on it.

I think your approach is better.  I'll drop my patch.

Tom

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

* Re: [RFA 2/5] Introduce ui_file_up and use it to remove cleanups
  2017-01-16 19:08     ` Tom Tromey
@ 2017-01-17  1:40       ` Pedro Alves
  2017-01-17 19:05         ` Tom Tromey
  0 siblings, 1 reply; 30+ messages in thread
From: Pedro Alves @ 2017-01-17  1:40 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

On 01/16/2017 07:08 PM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
> 
> Pedro> Urgh, guess you forgot/missed my 
> Pedro> palves/cxx-eliminate-cleanups branch ... :-(
> 
> Yeah.  I knew of it but forgot what was on it.
> 
> I think your approach is better.  I'll drop my patch.

Really sorry about the wasted effort.  :-(

As penance, I've spend a few hours tonight cleaning up the patch
a bit, splitting a few bits to separate patches, writing
ChangeLogs, etc.  I've posted it now.

Thanks,
Pedro Alves

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

* Re: [RFA 2/5] Introduce ui_file_up and use it to remove cleanups
  2017-01-17  1:40       ` Pedro Alves
@ 2017-01-17 19:05         ` Tom Tromey
  0 siblings, 0 replies; 30+ messages in thread
From: Tom Tromey @ 2017-01-17 19:05 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Tom Tromey, gdb-patches

Pedro> Really sorry about the wasted effort.  :-(

It's not a big deal.

Pedro> As penance, I've spend a few hours tonight cleaning up the patch
Pedro> a bit, splitting a few bits to separate patches, writing
Pedro> ChangeLogs, etc.  I've posted it now.

... but thank you for doing this.

Tom

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

* Re: [RFA 3/5] Introduce gdbpy_subclass and use it to simplify some  logic
  2017-01-15 13:43 ` [RFA 3/5] Introduce gdbpy_subclass and use it to simplify some logic Tom Tromey
@ 2017-01-24 20:21   ` Simon Marchi
  2017-02-09 11:44     ` Pedro Alves
  2017-02-09 13:00   ` Pedro Alves
  1 sibling, 1 reply; 30+ messages in thread
From: Simon Marchi @ 2017-01-24 20:21 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

On 2017-01-15 08:42, Tom Tromey wrote:
> This introduces a new typed variant of gdbpy_ref, called
> gdbpy_subclass.  This is then used to simplify logic in various parts
> of the Python layer; for example removing repeated error code or
> removing gotos.
> 
> I wouldn't mind a better than than "gdb_subclass".  One idea was to
> use gdb_ref with a default template parameter, and then change the
> existing uses of "gdb_ref" to "gdb_ref<>".

Do you mean gdbpy_ref and gdbpy_subclass?

I don't really like gdbpy_subclass, I think there should be "ref" in the 
name to be clear.  So it could be gdbpy_subclass_ref.  However, I find 
gdbpy_subclass_ref<gdbpy_breakpoint_object> a bit long.  As you may have 
seen in my version of the patch, I had decided to keep gdbpy_ref for 
PyObjects and introduce typedef for other types (gdbpy_inf_ref).  So I 
could see one called gdbpy_bp_ref.

Otherwise, I like gdbpy_ref<> and gdbpy_ref<gdbpy_breakpoint_object>.

The patch looked good to me otherwise (and confirmed that I still find 
refcounting difficult).

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

* Re: [RFA 1/5] Remove some ui_out-related cleanups from Python
  2017-01-16 11:19   ` Trevor Saunders
@ 2017-02-08 17:28     ` Pedro Alves
  2017-02-08 22:27       ` Pedro Alves
                         ` (2 more replies)
  0 siblings, 3 replies; 30+ messages in thread
From: Pedro Alves @ 2017-02-08 17:28 UTC (permalink / raw)
  To: Trevor Saunders, Tom Tromey; +Cc: gdb-patches

On 01/16/2017 11:30 AM, Trevor Saunders wrote:
>> +++ b/gdb/common/gdb_option.h
> 
> might be nice to put it in include/ but fine to do that later when
> something else actually wants it.

I've been thinking about putting all these utilities and
later-standards replacements we're coming up with in its own
directory/namespace.  I had thought of "gtl", for "gdb template
library", or "gnu template library" if other projects want to
reuse it.  (I think "gnu" is kind of taken by gnulib / libgnu.a)
One difficulty with putting such a thing at the top level is that
gdb relies on gnulib headers (that exist under gdb/) while
other toolchain components don't, yet, at least.

>> > +  /* These aren't deleted in std::optional, but it was simpler to
>> > +     delete them here, because currently the users of this class don't
>> > +     need them, and making them depend on the definition of T is
>> > +     somewhat complicated.  */
> I think you can make <type_traits> do most of it, but fair enough.
> 

Back around <https://sourceware.org/ml/gdb-patches/2016-11/msg00368.html>
I backported C++17's optional to C++11.  I still have that branch
somewhere locally...  /me finds it and pushes to github.  Here:

 https://github.com/palves/gdb/commits/palves/gdb-import-gcc-optional

This <https://github.com/palves/gdb/commit/880ab485c5873eb0a8ab1dca75e624ec2a064fe8>
contains the local changes I had made to to port it to C++11.  I was surprised
that other than portable type traits stuff that's missing in C++11, the
only thing that C++11 loses is constexpr-ness in some cases.  Now,
the result is of course much more code than what Tromey is proposing.
It probably does make sense to start with something simpler and
upgrade when/if we find a need.  OTOH, looks like the current patch
doesn't have accessors for the wrapped value, so it seems like we'll
at least need to be extend it in that direction in no time.

>> +  /* True if the object was ever emplaced.  */
>> +  bool m_instantiated;
>> +
>> +  /* The object.  */
>> +  union
>> +  {
>> +    struct { } m_dummy;
>> +    T m_item;
>> +  };
> 
> It doesn't matter yet, but space wise it would be better to put the bool
> last right? For example if sizeof(T) is 6, or if the optional is in some
> larger structure with a bool next.

Agreed.  Seems like that's what gcc's optional does.

I also agree with Simon's suggestion for renaming the file.

Patch LGTM with Trevor's and Simon's nits  addressed.

I wondered about making m_instantiated a char, so that optional<T> would
pack even better when sizeof or alignof T is small, and thus ends up being
no cost in those cases space-wise.  Though maybe that ends up being
worse / not so efficient generated code -wise, or GCC would do it too?
In any case, since GCC doesn't do that, if/when we ever move to C++17,
we'd lose that again anyway.

Thanks,
Pedro Alves

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

* Re: [RFA 1/5] Remove some ui_out-related cleanups from Python
  2017-02-08 17:28     ` Pedro Alves
@ 2017-02-08 22:27       ` Pedro Alves
  2017-02-08 23:05       ` Tom Tromey
  2017-02-10  6:47       ` Trevor Saunders
  2 siblings, 0 replies; 30+ messages in thread
From: Pedro Alves @ 2017-02-08 22:27 UTC (permalink / raw)
  To: Trevor Saunders, Tom Tromey; +Cc: gdb-patches

On 02/08/2017 05:28 PM, Pedro Alves wrote:

> I wondered about making m_instantiated a char, so that optional<T> would
> pack even better when sizeof or alignof T is small, and thus ends up being
> no cost in those cases space-wise.  Though maybe that ends up being
> worse / not so efficient generated code -wise, or GCC would do it too?
> In any case, since GCC doesn't do that, if/when we ever move to C++17,
> we'd lose that again anyway.

Eh, now that I check I see that sizeof bool == 1, and looking at GCC's sources,
I see it's like that for almost all ports.  Somehow I misremembered
this and thought it was sizeof(int).

Thanks,
Pedro Alves

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

* Re: [RFA 1/5] Remove some ui_out-related cleanups from Python
  2017-02-08 17:28     ` Pedro Alves
  2017-02-08 22:27       ` Pedro Alves
@ 2017-02-08 23:05       ` Tom Tromey
  2017-02-08 23:52         ` Pedro Alves
  2017-02-10  6:47       ` Trevor Saunders
  2 siblings, 1 reply; 30+ messages in thread
From: Tom Tromey @ 2017-02-08 23:05 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Trevor Saunders, Tom Tromey, gdb-patches

>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> One difficulty with putting such a thing at the top level is that
Pedro> gdb relies on gnulib headers (that exist under gdb/) while
Pedro> other toolchain components don't, yet, at least.

gnulib should just be at top-level, but I realize that's a pain.

Pedro> It probably does make sense to start with something simpler and
Pedro> upgrade when/if we find a need.  OTOH, looks like the current patch
Pedro> doesn't have accessors for the wrapped value, so it seems like we'll
Pedro> at least need to be extend it in that direction in no time.

I had forgotten about this code.  I don't mind using it instead.

Pedro> Patch LGTM with Trevor's and Simon's nits  addressed.

I have that done locally, but I've just been waiting until your series
lands, so I can rebase.  And then maybe the option stuff wouldn't be
needed anyway?  I don't remember the details from your branch, I was
just waiting for the big rebase to find out.

Tom

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

* Re: [RFA 1/5] Remove some ui_out-related cleanups from Python
  2017-02-08 23:05       ` Tom Tromey
@ 2017-02-08 23:52         ` Pedro Alves
  2017-02-09  4:34           ` Matt Rice
  0 siblings, 1 reply; 30+ messages in thread
From: Pedro Alves @ 2017-02-08 23:52 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Trevor Saunders, gdb-patches

On 02/08/2017 11:04 PM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
> 
> Pedro> One difficulty with putting such a thing at the top level is that
> Pedro> gdb relies on gnulib headers (that exist under gdb/) while
> Pedro> other toolchain components don't, yet, at least.
> 
> gnulib should just be at top-level, but I realize that's a pain.
> 
> Pedro> It probably does make sense to start with something simpler and
> Pedro> upgrade when/if we find a need.  OTOH, looks like the current patch
> Pedro> doesn't have accessors for the wrapped value, so it seems like we'll
> Pedro> at least need to be extend it in that direction in no time.
> 
> I had forgotten about this code.  I don't mind using it instead.

Let's start with your version, and leave the door open.  The version I have
would need a bit of cleaning, re-checking if GCC's implementation changed
meanwhile (ISTR there have been recent patches to address some DRs), check
that it compiles against clang and older gcc's (4.8, etc.).  I already have too
many balls in the air ATM.

> Pedro> Patch LGTM with Trevor's and Simon's nits  addressed.
> 
> I have that done locally, but I've just been waiting until your series
> lands, so I can rebase.  And then maybe the option stuff wouldn't be
> needed anyway?  I don't remember the details from your branch, I was
> just waiting for the big rebase to find out.

My series is all in master.  I think we'll still need your patch, since I only
touched ui_files.  I haven't done anything with ui_out list/tuple building.

Thanks,
Pedro Alves

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

* Re: [RFA 1/5] Remove some ui_out-related cleanups from Python
  2017-02-08 23:52         ` Pedro Alves
@ 2017-02-09  4:34           ` Matt Rice
  2017-02-09 12:48             ` Pedro Alves
  0 siblings, 1 reply; 30+ messages in thread
From: Matt Rice @ 2017-02-09  4:34 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Tom Tromey, Trevor Saunders, gdb-patches

On Wed, Feb 8, 2017 at 3:51 PM, Pedro Alves <palves@redhat.com> wrote:

>> Pedro> Patch LGTM with Trevor's and Simon's nits  addressed.
>>
>> I have that done locally, but I've just been waiting until your series
>> lands, so I can rebase.  And then maybe the option stuff wouldn't be
>> needed anyway?  I don't remember the details from your branch, I was
>> just waiting for the big rebase to find out.
>
> My series is all in master.  I think we'll still need your patch, since I only
> touched ui_files.  I haven't done anything with ui_out list/tuple building.

I haven't been following much lately due to programming mostly in
languages unsupported by gdb apologies if this has been hashed out
before.

Wondering if this would be a good opportunity to try and transition
ui_out_list construction into a more type safe manner, e.g.

result ==> variable "=" value
list ==> "[]" | "[" value ( "," value )* "]" | "[" result ( "," result )* "]"

currently when building a list, it didn't specify whether you were
declaring a list of results or a list of values, unless/until the
first value or result was added, and IIRC this | property wasn't
really explicitly enforced.

it'd be nice to migrate this to something like:
result_list, value_list, deprecated_unspecified_list, It would be nice
to know if/when you guys think it would be a convenient time to
introduce such a change so I could get back up to speed...

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

* Re: [RFA 3/5] Introduce gdbpy_subclass and use it to simplify some logic
  2017-01-24 20:21   ` Simon Marchi
@ 2017-02-09 11:44     ` Pedro Alves
  2017-02-09 18:52       ` Tom Tromey
  0 siblings, 1 reply; 30+ messages in thread
From: Pedro Alves @ 2017-02-09 11:44 UTC (permalink / raw)
  To: Simon Marchi, Tom Tromey; +Cc: gdb-patches

On 01/24/2017 08:21 PM, Simon Marchi wrote:

>> I wouldn't mind a better than than "gdb_subclass".  One idea was to
>> use gdb_ref with a default template parameter, and then change the
>> existing uses of "gdb_ref" to "gdb_ref<>".
> 
> Do you mean gdbpy_ref and gdbpy_subclass?
> 
> I don't really like gdbpy_subclass, I think there should be "ref" in the
> name to be clear.  So it could be gdbpy_subclass_ref.  However, I find
> gdbpy_subclass_ref<gdbpy_breakpoint_object> a bit long.  As you may have
> seen in my version of the patch, I had decided to keep gdbpy_ref for
> PyObjects and introduce typedef for other types (gdbpy_inf_ref).  So I
> could see one called gdbpy_bp_ref.
> 
> Otherwise, I like gdbpy_ref<> and gdbpy_ref<gdbpy_breakpoint_object>.
> 
> The patch looked good to me otherwise (and confirmed that I still find
> refcounting difficult).

I agree.  Simon's gdbpy_ref_base + typedef idea would work for me too.

Thanks,
Pedro Alves

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

* Re: [RFA 1/5] Remove some ui_out-related cleanups from Python
  2017-02-09  4:34           ` Matt Rice
@ 2017-02-09 12:48             ` Pedro Alves
  2017-02-09 12:51               ` Pedro Alves
  0 siblings, 1 reply; 30+ messages in thread
From: Pedro Alves @ 2017-02-09 12:48 UTC (permalink / raw)
  To: Matt Rice; +Cc: Tom Tromey, Trevor Saunders, gdb-patches

Hi Matt,

On 02/09/2017 04:34 AM, Matt Rice wrote:

> Wondering if this would be a good opportunity to try and transition
> ui_out_list construction into a more type safe manner, e.g.
> result ==> variable "=" value
> list ==> "[]" | "[" value ( "," value )* "]" | "[" result ( "," result )* "]"
> 
> currently when building a list, it didn't specify whether you were
> declaring a list of results or a list of values, unless/until the
> first value or result was added, and IIRC this | property wasn't
> really explicitly enforced.

More type-safety sounds good to me.  :-)

Can you give an example of a command that outputs a result list,
and an example of a command that outputs a value list?

> it'd be nice to migrate this to something like:
> result_list, value_list, deprecated_unspecified_list, It would be nice
> to know if/when you guys think it would be a convenient time to
> introduce such a change so I could get back up to speed...

Simon has already C++-fyed ui_out in master, so this seems like
a good time to me.

Thanks,
Pedro Alves

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

* Re: [RFA 1/5] Remove some ui_out-related cleanups from Python
  2017-02-09 12:48             ` Pedro Alves
@ 2017-02-09 12:51               ` Pedro Alves
  2017-02-09 15:46                 ` Matt Rice
  0 siblings, 1 reply; 30+ messages in thread
From: Pedro Alves @ 2017-02-09 12:51 UTC (permalink / raw)
  To: Matt Rice; +Cc: Tom Tromey, Trevor Saunders, gdb-patches

On 02/09/2017 12:48 PM, Pedro Alves wrote:
> Hi Matt,
> 
> On 02/09/2017 04:34 AM, Matt Rice wrote:
> 
>> Wondering if this would be a good opportunity to try and transition
>> ui_out_list construction into a more type safe manner, e.g.
>> result ==> variable "=" value
>> list ==> "[]" | "[" value ( "," value )* "]" | "[" result ( "," result )* "]"
>>
>> currently when building a list, it didn't specify whether you were
>> declaring a list of results or a list of values, unless/until the
>> first value or result was added, and IIRC this | property wasn't
>> really explicitly enforced.
> 
> More type-safety sounds good to me.  :-)
> 
> Can you give an example of a command that outputs a result list,
> and an example of a command that outputs a value list?
> 
>> it'd be nice to migrate this to something like:
>> result_list, value_list, deprecated_unspecified_list, It would be nice
>> to know if/when you guys think it would be a convenient time to
>> introduce such a change so I could get back up to speed...
> 
> Simon has already C++-fyed ui_out in master, so this seems like
> a good time to me.

Though, to be clear, IIUC, you're talking about changing
GDB internals without affecting the resulting MI output, right?

Thanks,
Pedro Alves

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

* Re: [RFA 4/5] Change one more spot to use gdbpy_ref
  2017-01-15 13:43 ` [RFA 4/5] Change one more spot to use gdbpy_ref Tom Tromey
@ 2017-02-09 12:52   ` Pedro Alves
  0 siblings, 0 replies; 30+ messages in thread
From: Pedro Alves @ 2017-02-09 12:52 UTC (permalink / raw)
  To: Tom Tromey, gdb-patches

On 01/15/2017 01:42 PM, Tom Tromey wrote:
> This patch changes one more spot in the Python layer to use gdbpy_ref.
> 
> 2017-01-15  Tom Tromey  <tom@tromey.com>
> 
> 	* python/py-prettyprint.c (pretty_print_one_value): Use
> 	gdbpy_ref.

LGTM.

Thanks,
Pedro Alves

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

* Re: [RFA 3/5] Introduce gdbpy_subclass and use it to simplify some logic
  2017-01-15 13:43 ` [RFA 3/5] Introduce gdbpy_subclass and use it to simplify some logic Tom Tromey
  2017-01-24 20:21   ` Simon Marchi
@ 2017-02-09 13:00   ` Pedro Alves
  1 sibling, 0 replies; 30+ messages in thread
From: Pedro Alves @ 2017-02-09 13:00 UTC (permalink / raw)
  To: Tom Tromey, gdb-patches

On 01/15/2017 01:42 PM, Tom Tromey wrote:

> diff --git a/gdb/python/py-ref.h b/gdb/python/py-ref.h
> index b2479bf..3430768 100644
> --- a/gdb/python/py-ref.h
> +++ b/gdb/python/py-ref.h
> @@ -1,6 +1,6 @@
>  /* Python reference-holding class
>  
> -   Copyright (C) 2016 Free Software Foundation, Inc.
> +   Copyright (C) 2016, 2017 Free Software Foundation, Inc.

No sure the end-year script converts this to a range automatically.
Better write 2016-2017 from the get go.


>  /* A gdb::ref_ptr that has been specialized for Python objects.  */
> -typedef gdb::ref_ptr<PyObject, gdbpy_ref_policy> gdbpy_ref;
> +typedef gdb::ref_ptr<PyObject, gdbpy_ref_policy<PyObject> > gdbpy_ref;

No need for the space in "> >" in C++11.

Other than the naming issue, this looks really nice to me.

Thanks,
Pedro Alves

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

* Re: [RFA 5/5] Remove some gotos from Python
  2017-01-15 13:43 ` [RFA 5/5] Remove some gotos from Python Tom Tromey
@ 2017-02-09 13:03   ` Pedro Alves
  0 siblings, 0 replies; 30+ messages in thread
From: Pedro Alves @ 2017-02-09 13:03 UTC (permalink / raw)
  To: Tom Tromey, gdb-patches

On 01/15/2017 01:42 PM, Tom Tromey wrote:
> This patch slightly refactors a couple of spots in the Python code to
> avoid some gotos.
> 
> 2017-01-15  Tom Tromey  <tom@tromey.com>
> 
> 	* python/python.c (do_start_initialization): New function, from
> 	_initialize_python.
> 	(_initialize_python): Call do_start_initialization.
> 	* python/py-linetable.c (ltpy_iternext): Use explicit returns, not
> 	goto.

OK.

Thanks,
Pedro Alves

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

* Re: [RFA 1/5] Remove some ui_out-related cleanups from Python
  2017-02-09 12:51               ` Pedro Alves
@ 2017-02-09 15:46                 ` Matt Rice
  2017-02-09 16:04                   ` Simon Marchi
  0 siblings, 1 reply; 30+ messages in thread
From: Matt Rice @ 2017-02-09 15:46 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Tom Tromey, Trevor Saunders, gdb-patches

On Thu, Feb 9, 2017 at 4:51 AM, Pedro Alves <palves@redhat.com> wrote:
> On 02/09/2017 12:48 PM, Pedro Alves wrote:
>> Hi Matt,
>>
>> On 02/09/2017 04:34 AM, Matt Rice wrote:
>>
>>> Wondering if this would be a good opportunity to try and transition
>>> ui_out_list construction into a more type safe manner, e.g.
>>> result ==> variable "=" value
>>> list ==> "[]" | "[" value ( "," value )* "]" | "[" result ( "," result )* "]"
>>>
>>> currently when building a list, it didn't specify whether you were
>>> declaring a list of results or a list of values, unless/until the
>>> first value or result was added, and IIRC this | property wasn't
>>> really explicitly enforced.
>>
>> More type-safety sounds good to me.  :-)
>>
>> Can you give an example of a command that outputs a result list,
>> and an example of a command that outputs a value list?
>>

There is a good example of a command which outputs both in the
documentation (in the example below "The -break-insert command")

interpreter-exec mi -break-insert main
interpreter-exec mi -break-insert -t foo
interpreter-exec mi -break-list

in the output of the -break-list command there is a list of results of the form:
body=[bkpt=... , bkpt=...]

https://sourceware.org/gdb/current/onlinedocs/gdb/GDB_002fMI-Breakpoint-Commands.html#g_t_002dbreak_002dinsert

I picked that one specifically because it shows the duplicate "bkpt" keys.
inside those bkpt={... thread-groups=["i1"] ... } should be a list of values.

interpreter-exec mi -list-thread-groups is a slightly easier to read
one that returns a list of values (tuples)
^done,groups=[tuple, ...]

Additionally of note is:
https://sourceware.org/gdb/current/onlinedocs/gdb/GDB_002fMI-Output-Syntax.html#GDB_002fMI-Output-Syntax
* New gdb/mi commands should only output lists containing values.

So, i suppose result_list should be some variation on
compat_only_result_list or some such...

>>> it'd be nice to migrate this to something like:
>>> result_list, value_list, deprecated_unspecified_list, It would be nice
>>> to know if/when you guys think it would be a convenient time to
>>> introduce such a change so I could get back up to speed...
>>
>> Simon has already C++-fyed ui_out in master, so this seems like
>> a good time to me.
>
> Though, to be clear, IIUC, you're talking about changing
> GDB internals without affecting the resulting MI output, right?

Yes, internals change only,

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

* Re: [RFA 1/5] Remove some ui_out-related cleanups from Python
  2017-02-09 15:46                 ` Matt Rice
@ 2017-02-09 16:04                   ` Simon Marchi
  0 siblings, 0 replies; 30+ messages in thread
From: Simon Marchi @ 2017-02-09 16:04 UTC (permalink / raw)
  To: Matt Rice; +Cc: Pedro Alves, Tom Tromey, Trevor Saunders, gdb-patches

On 2017-02-09 10:46, Matt Rice wrote:
> There is a good example of a command which outputs both in the
> documentation (in the example below "The -break-insert command")
> 
> interpreter-exec mi -break-insert main
> interpreter-exec mi -break-insert -t foo
> interpreter-exec mi -break-list
> 
> in the output of the -break-list command there is a list of results of 
> the form:
> body=[bkpt=... , bkpt=...]
> 
> https://sourceware.org/gdb/current/onlinedocs/gdb/GDB_002fMI-Breakpoint-Commands.html#g_t_002dbreak_002dinsert
> 
> I picked that one specifically because it shows the duplicate "bkpt" 
> keys.
> inside those bkpt={... thread-groups=["i1"] ... } should be a list of 
> values.
> 
> interpreter-exec mi -list-thread-groups is a slightly easier to read
> one that returns a list of values (tuples)
> ^done,groups=[tuple, ...]
> 
> Additionally of note is:
> https://sourceware.org/gdb/current/onlinedocs/gdb/GDB_002fMI-Output-Syntax.html#GDB_002fMI-Output-Syntax
> * New gdb/mi commands should only output lists containing values.
> 
> So, i suppose result_list should be some variation on
> compat_only_result_list or some such...

I like the idea of enforcing proper MI output through code.  One 
situation I know GDB outputs broken MI is with multiple locations 
breakpoints.  See:

   https://sourceware.org/bugzilla/show_bug.cgi?id=14733

It'd be nice if the MI building API didn't even let us do that.

>>>> it'd be nice to migrate this to something like:
>>>> result_list, value_list, deprecated_unspecified_list, It would be 
>>>> nice
>>>> to know if/when you guys think it would be a convenient time to
>>>> introduce such a change so I could get back up to speed...

Yes that would be great.  I would say that the most convenient time is 
when you have time to contribute to the project :).

About the naming, I think that "result_list" and "value_list" is 
confusing, since a list containing results is exactly what is 
deprecated.  What about just "tuple" and "list"?  According to the 
syntax and the note you mentioned, a tuple ({...}) will only accept 
results (foo=bar) and a list ([...]) will only accept values.  And we'd 
have deprecated_result_list to build lists containing results.

Simon

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

* Re: [RFA 3/5] Introduce gdbpy_subclass and use it to simplify some logic
  2017-02-09 11:44     ` Pedro Alves
@ 2017-02-09 18:52       ` Tom Tromey
  0 siblings, 0 replies; 30+ messages in thread
From: Tom Tromey @ 2017-02-09 18:52 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Simon Marchi, Tom Tromey, gdb-patches

>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

>> I don't really like gdbpy_subclass, I think there should be "ref" in the
>> name to be clear.  So it could be gdbpy_subclass_ref.  However, I find
>> gdbpy_subclass_ref<gdbpy_breakpoint_object> a bit long.  As you may have
>> seen in my version of the patch, I had decided to keep gdbpy_ref for
>> PyObjects and introduce typedef for other types (gdbpy_inf_ref).  So I
>> could see one called gdbpy_bp_ref.
>> 
>> Otherwise, I like gdbpy_ref<> and gdbpy_ref<gdbpy_breakpoint_object>.

Pedro> I agree.  Simon's gdbpy_ref_base + typedef idea would work for me too.

This patch would mean introducing 12 typedefs, most used (so far) in a
single spot.

That doesn't seem so great to me, so I think I will look at the
gdbpy_ref<> rename instead.

Tom

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

* Re: [RFA 1/5] Remove some ui_out-related cleanups from Python
  2017-02-08 17:28     ` Pedro Alves
  2017-02-08 22:27       ` Pedro Alves
  2017-02-08 23:05       ` Tom Tromey
@ 2017-02-10  6:47       ` Trevor Saunders
  2 siblings, 0 replies; 30+ messages in thread
From: Trevor Saunders @ 2017-02-10  6:47 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Tom Tromey, gdb-patches

On Wed, Feb 08, 2017 at 05:28:11PM +0000, Pedro Alves wrote:
> On 01/16/2017 11:30 AM, Trevor Saunders wrote:
> >> +++ b/gdb/common/gdb_option.h
> > 
> > might be nice to put it in include/ but fine to do that later when
> > something else actually wants it.
> 
> I've been thinking about putting all these utilities and
> later-standards replacements we're coming up with in its own
> directory/namespace.  I had thought of "gtl", for "gdb template
> library", or "gnu template library" if other projects want to
> reuse it.  (I think "gnu" is kind of taken by gnulib / libgnu.a)
> One difficulty with putting such a thing at the top level is that
> gdb relies on gnulib headers (that exist under gdb/) while
> other toolchain components don't, yet, at least.

yeah. I've been meaning to get to that too.  I think for some of these
things that may not matter, but for some I'm sure it does.

> >> > +  /* These aren't deleted in std::optional, but it was simpler to
> >> > +     delete them here, because currently the users of this class don't
> >> > +     need them, and making them depend on the definition of T is
> >> > +     somewhat complicated.  */
> > I think you can make <type_traits> do most of it, but fair enough.
> > 
> 
> Back around <https://sourceware.org/ml/gdb-patches/2016-11/msg00368.html>
> I backported C++17's optional to C++11.  I still have that branch
> somewhere locally...  /me finds it and pushes to github.  Here:
> 
>  https://github.com/palves/gdb/commits/palves/gdb-import-gcc-optional
> 
> This <https://github.com/palves/gdb/commit/880ab485c5873eb0a8ab1dca75e624ec2a064fe8>
> contains the local changes I had made to to port it to C++11.  I was surprised
> that other than portable type traits stuff that's missing in C++11, the
> only thing that C++11 loses is constexpr-ness in some cases.  Now,
> the result is of course much more code than what Tromey is proposing.
> It probably does make sense to start with something simpler and
> upgrade when/if we find a need.  OTOH, looks like the current patch
> doesn't have accessors for the wrapped value, so it seems like we'll
> at least need to be extend it in that direction in no time.

yeah, I think start small is a very reasonable approach, though I'd
agree you'll want to get at the wrapped value pretty soon.

Trev

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

end of thread, other threads:[~2017-02-10  6:47 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-15 13:43 [RFA 0/5] more cleanup removal in Python Tom Tromey
2017-01-15 13:43 ` [RFA 3/5] Introduce gdbpy_subclass and use it to simplify some logic Tom Tromey
2017-01-24 20:21   ` Simon Marchi
2017-02-09 11:44     ` Pedro Alves
2017-02-09 18:52       ` Tom Tromey
2017-02-09 13:00   ` Pedro Alves
2017-01-15 13:43 ` [RFA 4/5] Change one more spot to use gdbpy_ref Tom Tromey
2017-02-09 12:52   ` Pedro Alves
2017-01-15 13:43 ` [RFA 1/5] Remove some ui_out-related cleanups from Python Tom Tromey
2017-01-15 21:52   ` Simon Marchi
2017-01-16 16:13     ` Tom Tromey
2017-01-16 11:19   ` Trevor Saunders
2017-02-08 17:28     ` Pedro Alves
2017-02-08 22:27       ` Pedro Alves
2017-02-08 23:05       ` Tom Tromey
2017-02-08 23:52         ` Pedro Alves
2017-02-09  4:34           ` Matt Rice
2017-02-09 12:48             ` Pedro Alves
2017-02-09 12:51               ` Pedro Alves
2017-02-09 15:46                 ` Matt Rice
2017-02-09 16:04                   ` Simon Marchi
2017-02-10  6:47       ` Trevor Saunders
2017-01-15 13:43 ` [RFA 2/5] Introduce ui_file_up and use it to remove cleanups Tom Tromey
2017-01-16  9:59   ` Trevor Saunders
2017-01-16 17:58   ` Pedro Alves
2017-01-16 19:08     ` Tom Tromey
2017-01-17  1:40       ` Pedro Alves
2017-01-17 19:05         ` Tom Tromey
2017-01-15 13:43 ` [RFA 5/5] Remove some gotos from Python Tom Tromey
2017-02-09 13:03   ` Pedro Alves

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).