[RFA 6/8] Remove make_delete_ui_cleanup

[RFA 6/8] Remove make_delete_ui_cleanup

[Tom Tromey]

This removes new_ui and delete_ui in favor of ordinary 'new' and
'delete', and then removes make_delete_ui_cleanup in favor of
std::unique_ptr.

2017-09-30  Tom Tromey  <tom@tromey.com>

	* event-top.c (stdin_event_handler): Update.
	* main.c (captured_main_1): Update.
	* top.h (make_delete_ui_cleanup): Remove.
	(struct ui): Add constructor and destructor.
	(new_ui, delete_ui): Remove.
	* top.c (make_delete_ui_cleanup): Remove.
	(new_ui_command): Use std::unique_ptr.
	(delete_ui_cleanup): Remove.
	(ui::ui): Rename from new_ui.  Update.
	(free_ui): Remove.
	(ui::~ui): Rename from delete_ui.  Update.
---
 gdb/ChangeLog   |  14 ++++++++
 gdb/event-top.c |   2 +-
 gdb/main.c      |   2 +-
 gdb/top.c       | 101 ++++++++++++++++++++------------------------------------
 gdb/top.h       |  13 ++++----
 5 files changed, 57 insertions(+), 75 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index f3faf73..ad6da2d 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,19 @@
 2017-09-30  Tom Tromey  <tom@tromey.com>
 
+	* event-top.c (stdin_event_handler): Update.
+	* main.c (captured_main_1): Update.
+	* top.h (make_delete_ui_cleanup): Remove.
+	(struct ui): Add constructor and destructor.
+	(new_ui, delete_ui): Remove.
+	* top.c (make_delete_ui_cleanup): Remove.
+	(new_ui_command): Use std::unique_ptr.
+	(delete_ui_cleanup): Remove.
+	(ui::ui): Rename from new_ui.  Update.
+	(free_ui): Remove.
+	(ui::~ui): Rename from delete_ui.  Update.
+
+2017-09-30  Tom Tromey  <tom@tromey.com>
+
 	* symfile.c (load_progress): Use gdb::byte_vector.
 
 2017-09-30  Tom Tromey  <tom@tromey.com>
diff --git a/gdb/event-top.c b/gdb/event-top.c
index 54fe471..43e2a27 100644
--- a/gdb/event-top.c
+++ b/gdb/event-top.c
@@ -487,7 +487,7 @@ stdin_event_handler (int error, gdb_client_data client_data)
       else
 	{
 	  /* Simply delete the UI.  */
-	  delete_ui (ui);
+	  delete ui;
 	}
     }
   else
diff --git a/gdb/main.c b/gdb/main.c
index f174a24..79f14b7 100644
--- a/gdb/main.c
+++ b/gdb/main.c
@@ -528,7 +528,7 @@ captured_main_1 (struct captured_main_args *context)
   setvbuf (stderr, NULL, _IONBF, BUFSIZ);
 #endif
 
-  main_ui = new_ui (stdin, stdout, stderr);
+  main_ui = new ui (stdin, stdout, stderr);
   current_ui = main_ui;
 
   gdb_stdtargerr = gdb_stderr;	/* for moment */
diff --git a/gdb/top.c b/gdb/top.c
index 56117a3..7efc3d5 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -249,91 +249,62 @@ static int highest_ui_num;
 
 /* See top.h.  */
 
-struct ui *
-new_ui (FILE *instream, FILE *outstream, FILE *errstream)
+ui::ui (FILE *instream_, FILE *outstream_, FILE *errstream_)
+  : next (nullptr),
+    num (++highest_ui_num),
+    call_readline (nullptr),
+    input_handler (nullptr),
+    command_editing (0),
+    interp_info (nullptr),
+    async (0),
+    secondary_prompt_depth (0),
+    stdin_stream (instream_),
+    instream (instream_),
+    outstream (outstream_),
+    errstream (errstream_),
+    input_fd (fileno (instream)),
+    input_interactive_p (ISATTY (instream)),
+    prompt_state (PROMPT_NEEDED),
+    m_gdb_stdout (new stdio_file (outstream)),
+    m_gdb_stdin (new stdio_file (instream)),
+    m_gdb_stderr (new stderr_file (errstream)),
+    m_gdb_stdlog (m_gdb_stderr),
+    m_current_uiout (nullptr)
 {
-  struct ui *ui;
-
-  ui = XCNEW (struct ui);
-
-  ui->num = ++highest_ui_num;
-  ui->stdin_stream = instream;
-  ui->instream = instream;
-  ui->outstream = outstream;
-  ui->errstream = errstream;
-
-  ui->input_fd = fileno (ui->instream);
-
-  ui->input_interactive_p = ISATTY (ui->instream);
-
-  ui->m_gdb_stdin = new stdio_file (ui->instream);
-  ui->m_gdb_stdout = new stdio_file (ui->outstream);
-  ui->m_gdb_stderr = new stderr_file (ui->errstream);
-  ui->m_gdb_stdlog = ui->m_gdb_stderr;
-
-  ui->prompt_state = PROMPT_NEEDED;
+  buffer_init (&line_buffer);
 
   if (ui_list == NULL)
-    ui_list = ui;
+    ui_list = this;
   else
     {
       struct ui *last;
 
       for (last = ui_list; last->next != NULL; last = last->next)
 	;
-      last->next = ui;
+      last->next = this;
     }
-
-  return ui;
-}
-
-static void
-free_ui (struct ui *ui)
-{
-  delete ui->m_gdb_stdin;
-  delete ui->m_gdb_stdout;
-  delete ui->m_gdb_stderr;
-
-  xfree (ui);
 }
 
-void
-delete_ui (struct ui *todel)
+ui::~ui ()
 {
   struct ui *ui, *uiprev;
 
   uiprev = NULL;
 
   for (ui = ui_list; ui != NULL; uiprev = ui, ui = ui->next)
-    if (ui == todel)
+    if (ui == this)
       break;
 
   gdb_assert (ui != NULL);
 
   if (uiprev != NULL)
-    uiprev->next = ui->next;
+    uiprev->next = next;
   else
-    ui_list = ui->next;
-
-  free_ui (ui);
-}
-
-/* Cleanup that deletes a UI.  */
-
-static void
-delete_ui_cleanup (void *void_ui)
-{
-  struct ui *ui = (struct ui *) void_ui;
+    ui_list = next;
 
-  delete_ui (ui);
-}
-
-/* See top.h.  */
-
-struct cleanup *
-make_delete_ui_cleanup (struct ui *ui)
-{
-  return make_cleanup (delete_ui_cleanup, ui);
+  delete m_gdb_stdin;
+  delete m_gdb_stdout;
+  delete m_gdb_stderr;
 }
 
 /* Open file named NAME for read/write, making sure not to make it the
@@ -356,7 +327,6 @@ open_terminal_stream (const char *name)
 static void
 new_ui_command (const char *args, int from_tty)
 {
-  struct ui *ui;
   struct interp *interp;
   gdb_file_up stream[3];
   int i;
@@ -364,7 +334,6 @@ new_ui_command (const char *args, int from_tty)
   int argc;
   const char *interpreter_name;
   const char *tty_name;
-  struct cleanup *failure_chain;
 
   dont_repeat ();
 
@@ -385,12 +354,12 @@ new_ui_command (const char *args, int from_tty)
     for (i = 0; i < 3; i++)
       stream[i] = open_terminal_stream (tty_name);
 
-    ui = new_ui (stream[0].get (), stream[1].get (), stream[2].get ());
-    failure_chain = make_cleanup (delete_ui_cleanup, ui);
+    std::unique_ptr<ui> ui
+      (new struct ui (stream[0].get (), stream[1].get (), stream[2].get ()));
 
     ui->async = 1;
 
-    current_ui = ui;
+    current_ui = ui.get ();
 
     set_top_level_interpreter (interpreter_name);
 
@@ -401,7 +370,7 @@ new_ui_command (const char *args, int from_tty)
     stream[1].release ();
     stream[2].release ();
 
-    discard_cleanups (failure_chain);
+    ui.release ();
   }
 
   printf_unfiltered ("New UI allocated\n");
diff --git a/gdb/top.h b/gdb/top.h
index 6b66083..99ba010 100644
--- a/gdb/top.h
+++ b/gdb/top.h
@@ -54,6 +54,12 @@ enum prompt_state
 
 struct ui
 {
+  /* Create a new UI.  */
+  ui (FILE *instream, FILE *outstream, FILE *errstream);
+  ~ui ();
+
+  DISABLE_COPY_AND_ASSIGN (ui);
+
   /* Pointer to next in singly-linked list.  */
   struct ui *next;
 
@@ -203,13 +209,6 @@ public:
 #define ALL_UIS(UI)				\
   for (UI = ui_list; UI; UI = UI->next)		\
 
-/* Create a new UI.  */
-extern struct ui *new_ui (FILE *instream, FILE *outstream, FILE *errstream);
-extern void delete_ui (struct ui *todel);
-
-/* Cleanup that deletes a UI.  */
-extern struct cleanup *make_delete_ui_cleanup (struct ui *ui);
-
 /* Register the UI's input file descriptor in the event loop.  */
 extern void ui_register_input_event_handler (struct ui *ui);
 
-- 
2.9.5

[RFA 8/8] Use std::string in info_symbol_command

[Tom Tromey]

This removes a cleanup by using std::string in info_symbol_command.

2017-09-30  Tom Tromey  <tom@tromey.com>

	* printcmd.c (info_symbol_command): Use std::string.
---
 gdb/ChangeLog  |  4 ++++
 gdb/printcmd.c | 16 +++++++---------
 2 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 293c796..20fa08c 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,9 @@
 2017-09-30  Tom Tromey  <tom@tromey.com>
 
+	* printcmd.c (info_symbol_command): Use std::string.
+
+2017-09-30  Tom Tromey  <tom@tromey.com>
+
 	* top.c (gdb_safe_append_history): Use std::string.
 
 2017-09-30  Tom Tromey  <tom@tromey.com>
diff --git a/gdb/printcmd.c b/gdb/printcmd.c
index a254e3a..994259d 100644
--- a/gdb/printcmd.c
+++ b/gdb/printcmd.c
@@ -1314,7 +1314,7 @@ info_symbol_command (char *arg, int from_tty)
 	    = lookup_minimal_symbol_by_pc_section (sect_addr, osect).minsym))
       {
 	const char *obj_name, *mapped, *sec_name, *msym_name;
-	char *loc_string;
+	const char *loc_string;
 	struct cleanup *old_chain;
 
 	matches = 1;
@@ -1325,14 +1325,14 @@ info_symbol_command (char *arg, int from_tty)
 
 	/* Don't print the offset if it is zero.
 	   We assume there's no need to handle i18n of "sym + offset".  */
+	std::string string_holder;
 	if (offset)
-	  loc_string = xstrprintf ("%s + %u", msym_name, offset);
+	  {
+	    string_holder = string_printf ("%s + %u", msym_name, offset);
+	    loc_string = string_holder.c_str ();
+	  }
 	else
-	  loc_string = xstrprintf ("%s", msym_name);
-
-	/* Use a cleanup to free loc_string in case the user quits
-	   a pagination request inside printf_filtered.  */
-	old_chain = make_cleanup (xfree, loc_string);
+	  loc_string = msym_name;
 
 	gdb_assert (osect->objfile && objfile_name (osect->objfile));
 	obj_name = objfile_name (osect->objfile);
@@ -1370,8 +1370,6 @@ info_symbol_command (char *arg, int from_tty)
 	    else
 	      printf_filtered (_("%s in section %s\n"),
 			       loc_string, sec_name);
-
-	do_cleanups (old_chain);
       }
   }
   if (matches == 0)
-- 
2.9.5

[RFA 4/8] Remove unused declarations

[Tom Tromey]

This removes some unused cleanup declarations.

2017-09-30  Tom Tromey  <tom@tromey.com>

	* mi/mi-main.c (mi_cmd_trace_frame_collected): Remove unused
	declaration.
	* printcmd.c (x_command): Remove unused declaration.
	* symfile.c (symbol_file_command): Remove unused declaration.
---
 gdb/ChangeLog    | 7 +++++++
 gdb/mi/mi-main.c | 1 -
 gdb/printcmd.c   | 1 -
 gdb/symfile.c    | 1 -
 4 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 6744ac0..ba841f8 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,12 @@
 2017-09-30  Tom Tromey  <tom@tromey.com>
 
+	* mi/mi-main.c (mi_cmd_trace_frame_collected): Remove unused
+	declaration.
+	* printcmd.c (x_command): Remove unused declaration.
+	* symfile.c (symbol_file_command): Remove unused declaration.
+
+2017-09-30  Tom Tromey  <tom@tromey.com>
+
 	* utils.c (internal_vproblem): Use std::string.
 	(defaulted_query): Likewise.
 
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index 4a61efd..83b1fcf 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -2730,7 +2730,6 @@ mi_cmd_trace_frame_collected (const char *command, char **argv, int argc)
 
   /* Trace state variables.  */
   {
-    struct cleanup *cleanups;
     int tvar;
     int i;
 
diff --git a/gdb/printcmd.c b/gdb/printcmd.c
index de1f76c..a254e3a 100644
--- a/gdb/printcmd.c
+++ b/gdb/printcmd.c
@@ -1606,7 +1606,6 @@ static void
 x_command (char *exp, int from_tty)
 {
   struct format_data fmt;
-  struct cleanup *old_chain;
   struct value *val;
 
   fmt.format = last_format ? last_format : 'x';
diff --git a/gdb/symfile.c b/gdb/symfile.c
index c6d657b..f6bc378 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -1629,7 +1629,6 @@ symbol_file_command (const char *args, int from_tty)
     {
       objfile_flags flags = OBJF_USERLOADED;
       symfile_add_flags add_flags = 0;
-      struct cleanup *cleanups;
       char *name = NULL;
 
       if (from_tty)
-- 
2.9.5

[RFA 0/8] more cleanup removal

[Tom Tromey]

This series removes more cleanups from gdb.  I think each patch speaks
for itself.  They should all be pretty straightforward.

Regression tested by the build bot.

After this, by the "grep 'struct cleanup'" metric, gdb has less than
500 remaining spots to fix.

Tom

[RFA 3/8] Use std::string in utils.c

[Tom Tromey]

This converts internal_vproblem and defaulted_query to use
std::string.

2017-09-30  Tom Tromey  <tom@tromey.com>

	* utils.c (internal_vproblem): Use std::string.
	(defaulted_query): Likewise.
---
 gdb/ChangeLog |  5 +++++
 gdb/utils.c   | 45 ++++++++++++++++++++-------------------------
 2 files changed, 25 insertions(+), 25 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index a17a83c..6744ac0 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,10 @@
 2017-09-30  Tom Tromey  <tom@tromey.com>
 
+	* utils.c (internal_vproblem): Use std::string.
+	(defaulted_query): Likewise.
+
+2017-09-30  Tom Tromey  <tom@tromey.com>
+
 	* guile/scm-ports.c (ioscm_with_output_to_port_worker): Update.
 	* top.c (execute_command_to_string): Update.
 	* utils.c (make_cleanup_restore_page_info): Remove.
diff --git a/gdb/utils.c b/gdb/utils.c
index 0c59b4e..7595c21 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -392,8 +392,7 @@ internal_vproblem (struct internal_problem *problem,
   static int dejavu;
   int quit_p;
   int dump_core_p;
-  char *reason;
-  struct cleanup *cleanup = make_cleanup (null_cleanup, NULL);
+  std::string reason;
 
   /* Don't allow infinite error/warning recursion.  */
   {
@@ -429,18 +428,17 @@ internal_vproblem (struct internal_problem *problem,
     char *msg;
 
     msg = xstrvprintf (fmt, ap);
-    reason = xstrprintf ("%s:%d: %s: %s\n"
-			 "A problem internal to GDB has been detected,\n"
-			 "further debugging may prove unreliable.",
-			 file, line, problem->name, msg);
+    reason = string_printf ("%s:%d: %s: %s\n"
+			    "A problem internal to GDB has been detected,\n"
+			    "further debugging may prove unreliable.",
+			    file, line, problem->name, msg);
     xfree (msg);
-    make_cleanup (xfree, reason);
   }
 
   /* Fall back to abort_with_message if gdb_stderr is not set up.  */
   if (current_ui == NULL)
     {
-      fputs (reason, stderr);
+      fputs (reason.c_str (), stderr);
       abort_with_message ("\n");
     }
 
@@ -458,7 +456,7 @@ internal_vproblem (struct internal_problem *problem,
   if (problem->should_quit != internal_problem_ask
       || !confirm
       || !filtered_printing_initialized ())
-    fprintf_unfiltered (gdb_stderr, "%s\n", reason);
+    fprintf_unfiltered (gdb_stderr, "%s\n", reason.c_str ());
 
   if (problem->should_quit == internal_problem_ask)
     {
@@ -468,7 +466,8 @@ internal_vproblem (struct internal_problem *problem,
       if (!confirm || !filtered_printing_initialized ())
 	quit_p = 1;
       else
-        quit_p = query (_("%s\nQuit this debugging session? "), reason);
+        quit_p = query (_("%s\nQuit this debugging session? "),
+			reason.c_str ());
     }
   else if (problem->should_quit == internal_problem_yes)
     quit_p = 1;
@@ -485,7 +484,7 @@ internal_vproblem (struct internal_problem *problem,
 
   if (problem->should_dump_core == internal_problem_ask)
     {
-      if (!can_dump_core_warn (LIMIT_MAX, reason))
+      if (!can_dump_core_warn (LIMIT_MAX, reason.c_str ()))
 	dump_core_p = 0;
       else if (!filtered_printing_initialized ())
 	dump_core_p = 1;
@@ -494,11 +493,12 @@ internal_vproblem (struct internal_problem *problem,
 	  /* Default (yes/batch case) is to dump core.  This leaves a GDB
 	     `dropping' so that it is easier to see that something went
 	     wrong in GDB.  */
-	  dump_core_p = query (_("%s\nCreate a core file of GDB? "), reason);
+	  dump_core_p = query (_("%s\nCreate a core file of GDB? "),
+			       reason.c_str ());
 	}
     }
   else if (problem->should_dump_core == internal_problem_yes)
-    dump_core_p = can_dump_core_warn (LIMIT_MAX, reason);
+    dump_core_p = can_dump_core_warn (LIMIT_MAX, reason.c_str ());
   else if (problem->should_dump_core == internal_problem_no)
     dump_core_p = 0;
   else
@@ -523,7 +523,6 @@ internal_vproblem (struct internal_problem *problem,
     }
 
   dejavu = 0;
-  do_cleanups (cleanup);
 }
 
 static struct internal_problem internal_error_problem = {
@@ -915,8 +914,6 @@ defaulted_query (const char *ctlstr, const char defchar, va_list args)
   int def_value;
   char def_answer, not_def_answer;
   const char *y_string, *n_string;
-  char *question, *prompt;
-  struct cleanup *old_chain;
 
   /* Set up according to which answer is the default.  */
   if (defchar == '\0')
@@ -978,13 +975,12 @@ defaulted_query (const char *ctlstr, const char defchar, va_list args)
     }
 
   /* Format the question outside of the loop, to avoid reusing args.  */
-  question = xstrvprintf (ctlstr, args);
-  old_chain = make_cleanup (xfree, question);
-  prompt = xstrprintf (_("%s%s(%s or %s) %s"),
-		      annotation_level > 1 ? "\n\032\032pre-query\n" : "",
-		      question, y_string, n_string,
-		      annotation_level > 1 ? "\n\032\032query\n" : "");
-  make_cleanup (xfree, prompt);
+  std::string question = string_vprintf (ctlstr, args);
+  std::string prompt
+    = string_printf (_("%s%s(%s or %s) %s"),
+		     annotation_level > 1 ? "\n\032\032pre-query\n" : "",
+		     question.c_str (), y_string, n_string,
+		     annotation_level > 1 ? "\n\032\032query\n" : "");
 
   /* Used to add duration we waited for user to respond to
      prompt_for_continue_wait_time.  */
@@ -998,7 +994,7 @@ defaulted_query (const char *ctlstr, const char defchar, va_list args)
       char *response, answer;
 
       gdb_flush (gdb_stdout);
-      response = gdb_readline_wrapper (prompt);
+      response = gdb_readline_wrapper (prompt.c_str ());
 
       if (response == NULL)	/* C-d  */
 	{
@@ -1038,7 +1034,6 @@ defaulted_query (const char *ctlstr, const char defchar, va_list args)
 
   if (annotation_level > 1)
     printf_filtered (("\n\032\032post-query\n"));
-  do_cleanups (old_chain);
   return retval;
 }
 \f
-- 
2.9.5

[RFA 7/8] Use std::string in gdb_safe_append_history

[Tom Tromey]

This removes a cleanup by using std::string in
gdb_safe_append_history.

2017-09-30  Tom Tromey  <tom@tromey.com>

	* top.c (gdb_safe_append_history): Use std::string.
---
 gdb/ChangeLog |  4 ++++
 gdb/top.c     | 24 ++++++++++--------------
 2 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index ad6da2d..293c796 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,9 @@
 2017-09-30  Tom Tromey  <tom@tromey.com>
 
+	* top.c (gdb_safe_append_history): Use std::string.
+
+2017-09-30  Tom Tromey  <tom@tromey.com>
+
 	* event-top.c (stdin_event_handler): Update.
 	* main.c (captured_main_1): Update.
 	* top.h (make_delete_ui_cleanup): Remove.
diff --git a/gdb/top.c b/gdb/top.c
index 7efc3d5..af27fcb 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -1094,19 +1094,16 @@ static void
 gdb_safe_append_history (void)
 {
   int ret, saved_errno;
-  char *local_history_filename;
-  struct cleanup *old_chain;
 
-  local_history_filename
-    = xstrprintf ("%s-gdb%ld~", history_filename, (long) getpid ());
-  old_chain = make_cleanup (xfree, local_history_filename);
+  std::string local_history_filename
+    = string_printf ("%s-gdb%ld~", history_filename, (long) getpid ());
 
-  ret = rename (history_filename, local_history_filename);
+  ret = rename (history_filename, local_history_filename.c_str ());
   saved_errno = errno;
   if (ret < 0 && saved_errno != ENOENT)
     {
       warning (_("Could not rename %s to %s: %s"),
-	       history_filename, local_history_filename,
+	       history_filename, local_history_filename.c_str (),
 	       safe_strerror (saved_errno));
     }
   else
@@ -1122,24 +1119,23 @@ gdb_safe_append_history (void)
 	     to move it back anyway.  Otherwise a global history file would
 	     never get created!  */
 	   gdb_assert (saved_errno == ENOENT);
-	   write_history (local_history_filename);
+	   write_history (local_history_filename.c_str ());
 	}
       else
 	{
-	  append_history (command_count, local_history_filename);
+	  append_history (command_count, local_history_filename.c_str ());
 	  if (history_is_stifled ())
-	    history_truncate_file (local_history_filename, history_max_entries);
+	    history_truncate_file (local_history_filename.c_str (),
+				   history_max_entries);
 	}
 
-      ret = rename (local_history_filename, history_filename);
+      ret = rename (local_history_filename.c_str (), history_filename);
       saved_errno = errno;
       if (ret < 0 && saved_errno != EEXIST)
         warning (_("Could not rename %s to %s: %s"),
-		 local_history_filename, history_filename,
+		 local_history_filename.c_str (), history_filename,
 		 safe_strerror (saved_errno));
     }
-
-  do_cleanups (old_chain);
 }
 
 /* Read one line from the command input stream `instream' into a local
-- 
2.9.5

[RFA 2/8] Remove set_batch_flag_and_make_cleanup_restore_page_info

[Tom Tromey]

This removes set_batch_flag_and_make_cleanup_restore_page_info and
make_cleanup_restore_page_info in favor of a new RAII class.  This
then allows for the removal of make_cleanup_restore_uinteger and
make_cleanup_restore_integer

ChangeLog
2017-09-30  Tom Tromey  <tom@tromey.com>

	* guile/scm-ports.c (ioscm_with_output_to_port_worker): Update.
	* top.c (execute_command_to_string): Update.
	* utils.c (make_cleanup_restore_page_info): Remove.
	(do_restore_page_info_cleanup): Remove.
	(set_batch_flag_and_restore_page_info):
	New.
	(make_cleanup_restore_page_info): Remove.
	(set_batch_flag_and_make_cleanup_restore_page_info): Remove.
	(~set_batch_flag_and_restore_page_info): New
	(make_cleanup_restore_uinteger): Remove.
	(make_cleanup_restore_integer): Remove.
	(struct restore_integer_closure): Remove.
	(restore_integer): Remove.
	* utils.h (struct set_batch_flag_and_restore_page_info): New
	class.
	(set_batch_flag_and_make_cleanup_restore_page_info): Remove.
	(make_cleanup_restore_page_info): Remove.
	(make_cleanup_restore_uinteger) Remove.
	(make_cleanup_restore_integer) Remove.
---
 gdb/ChangeLog         | 22 ++++++++++++++
 gdb/guile/scm-ports.c |  5 +---
 gdb/top.c             |  6 +---
 gdb/utils.c           | 81 ++++++++-------------------------------------------
 gdb/utils.h           | 27 +++++++++++++----
 5 files changed, 57 insertions(+), 84 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index a415c18..a17a83c 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,27 @@
 2017-09-30  Tom Tromey  <tom@tromey.com>
 
+	* guile/scm-ports.c (ioscm_with_output_to_port_worker): Update.
+	* top.c (execute_command_to_string): Update.
+	* utils.c (make_cleanup_restore_page_info): Remove.
+	(do_restore_page_info_cleanup): Remove.
+	(set_batch_flag_and_restore_page_info):
+	New.
+	(make_cleanup_restore_page_info): Remove.
+	(set_batch_flag_and_make_cleanup_restore_page_info): Remove.
+	(~set_batch_flag_and_restore_page_info): New
+	(make_cleanup_restore_uinteger): Remove.
+	(make_cleanup_restore_integer): Remove.
+	(struct restore_integer_closure): Remove.
+	(restore_integer): Remove.
+	* utils.h (struct set_batch_flag_and_restore_page_info): New
+	class.
+	(set_batch_flag_and_make_cleanup_restore_page_info): Remove.
+	(make_cleanup_restore_page_info): Remove.
+	(make_cleanup_restore_uinteger) Remove.
+	(make_cleanup_restore_integer) Remove.
+
+2017-09-30  Tom Tromey  <tom@tromey.com>
+
 	* record-full.h (record_full_gdb_operation_disable_set): Return
 	scoped_restore_tmpl<int>.
 	* infrun.c (adjust_pc_after_break): Update.
diff --git a/gdb/guile/scm-ports.c b/gdb/guile/scm-ports.c
index 78187c4..a7c0bd4 100644
--- a/gdb/guile/scm-ports.c
+++ b/gdb/guile/scm-ports.c
@@ -461,7 +461,6 @@ static SCM
 ioscm_with_output_to_port_worker (SCM port, SCM thunk, enum oport oport,
 				  const char *func_name)
 {
-  struct cleanup *cleanups;
   SCM result;
 
   SCM_ASSERT_TYPE (gdbscm_is_true (scm_output_port_p (port)), port,
@@ -469,7 +468,7 @@ ioscm_with_output_to_port_worker (SCM port, SCM thunk, enum oport oport,
   SCM_ASSERT_TYPE (gdbscm_is_true (scm_thunk_p (thunk)), thunk,
 		   SCM_ARG2, func_name, _("thunk"));
 
-  cleanups = set_batch_flag_and_make_cleanup_restore_page_info ();
+  set_batch_flag_and_restore_page_info save_page_info;
 
   scoped_restore restore_async = make_scoped_restore (&current_ui->async, 0);
 
@@ -493,8 +492,6 @@ ioscm_with_output_to_port_worker (SCM port, SCM thunk, enum oport oport,
     result = gdbscm_safe_call_0 (thunk, NULL);
   }
 
-  do_cleanups (cleanups);
-
   if (gdbscm_is_exception (result))
     gdbscm_throw (result);
 
diff --git a/gdb/top.c b/gdb/top.c
index 4fc987c..56117a3 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -666,11 +666,9 @@ execute_command (char *p, int from_tty)
 std::string
 execute_command_to_string (char *p, int from_tty)
 {
-  struct cleanup *cleanup;
-
   /* GDB_STDOUT should be better already restored during these
      restoration callbacks.  */
-  cleanup = set_batch_flag_and_make_cleanup_restore_page_info ();
+  set_batch_flag_and_restore_page_info save_page_info;
 
   scoped_restore save_async = make_scoped_restore (&current_ui->async, 0);
 
@@ -694,8 +692,6 @@ execute_command_to_string (char *p, int from_tty)
     execute_command (p, from_tty);
   }
 
-  do_cleanups (cleanup);
-
   return std::move (str_file.string ());
 }
 
diff --git a/gdb/utils.c b/gdb/utils.c
index b2e0813..0c59b4e 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -150,44 +150,6 @@ make_cleanup_free_section_addr_info (struct section_addr_info *addrs)
   return make_cleanup (do_free_section_addr_info, addrs);
 }
 
-struct restore_integer_closure
-{
-  int *variable;
-  int value;
-};
-
-static void
-restore_integer (void *p)
-{
-  struct restore_integer_closure *closure
-    = (struct restore_integer_closure *) p;
-
-  *(closure->variable) = closure->value;
-}
-
-/* Remember the current value of *VARIABLE and make it restored when
-   the cleanup is run.  */
-
-struct cleanup *
-make_cleanup_restore_integer (int *variable)
-{
-  struct restore_integer_closure *c = XNEW (struct restore_integer_closure);
-
-  c->variable = variable;
-  c->value = *variable;
-
-  return make_cleanup_dtor (restore_integer, (void *) c, xfree);
-}
-
-/* Remember the current value of *VARIABLE and make it restored when
-   the cleanup is run.  */
-
-struct cleanup *
-make_cleanup_restore_uinteger (unsigned int *variable)
-{
-  return make_cleanup_restore_integer ((int *) variable);
-}
-
 /* Helper for make_cleanup_unpush_target.  */
 
 static void
@@ -1464,42 +1426,23 @@ filtered_printing_initialized (void)
   return wrap_buffer != NULL;
 }
 
-/* Helper for make_cleanup_restore_page_info.  */
-
-static void
-do_restore_page_info_cleanup (void *arg)
-{
-  set_screen_size ();
-  set_width ();
-}
-
-/* Provide cleanup for restoring the terminal size.  */
-
-struct cleanup *
-make_cleanup_restore_page_info (void)
+set_batch_flag_and_restore_page_info::set_batch_flag_and_restore_page_info ()
+  : m_save_lines_per_page (lines_per_page),
+    m_save_chars_per_line (chars_per_line),
+    m_save_batch_flag (batch_flag)
 {
-  struct cleanup *back_to;
-
-  back_to = make_cleanup (do_restore_page_info_cleanup, NULL);
-  make_cleanup_restore_uinteger (&lines_per_page);
-  make_cleanup_restore_uinteger (&chars_per_line);
-
-  return back_to;
+  batch_flag = 1;
+  init_page_info ();
 }
 
-/* Temporarily set BATCH_FLAG and the associated unlimited terminal size.
-   Provide cleanup for restoring the original state.  */
-
-struct cleanup *
-set_batch_flag_and_make_cleanup_restore_page_info (void)
+set_batch_flag_and_restore_page_info::~set_batch_flag_and_restore_page_info ()
 {
-  struct cleanup *back_to = make_cleanup_restore_page_info ();
-  
-  make_cleanup_restore_integer (&batch_flag);
-  batch_flag = 1;
-  init_page_info ();
+  batch_flag = m_save_batch_flag;
+  chars_per_line = m_save_chars_per_line;
+  lines_per_page = m_save_lines_per_page;
 
-  return back_to;
+  set_screen_size ();
+  set_width ();
 }
 
 /* Set the screen size based on LINES_PER_PAGE and CHARS_PER_LINE.  */
diff --git a/gdb/utils.h b/gdb/utils.h
index 7b45cc8..022af51 100644
--- a/gdb/utils.h
+++ b/gdb/utils.h
@@ -211,9 +211,6 @@ extern struct cleanup *(make_cleanup_free_section_addr_info
 
 /* For make_cleanup_close see common/filestuff.h.  */
 
-extern struct cleanup *make_cleanup_restore_integer (int *variable);
-extern struct cleanup *make_cleanup_restore_uinteger (unsigned int *variable);
-
 struct target_ops;
 extern struct cleanup *make_cleanup_unpush_target (struct target_ops *ops);
 
@@ -236,9 +233,27 @@ extern void free_current_contents (void *);
 
 extern void init_page_info (void);
 
-extern struct cleanup *make_cleanup_restore_page_info (void);
-extern struct cleanup *
-  set_batch_flag_and_make_cleanup_restore_page_info (void);
+/* Temporarily set BATCH_FLAG and the associated unlimited terminal size.
+   Restore when destroyed.  */
+
+struct set_batch_flag_and_restore_page_info
+{
+public:
+
+  set_batch_flag_and_restore_page_info ();
+  ~set_batch_flag_and_restore_page_info ();
+
+  DISABLE_COPY_AND_ASSIGN (set_batch_flag_and_restore_page_info);
+
+private:
+
+  /* Note that this doesn't use scoped_restore, because it's important
+     to control the ordering of operations in the destruction, and it
+     was simpler to avoid introducing a new ad hoc class.  */
+  unsigned m_save_lines_per_page;
+  unsigned m_save_chars_per_line;
+  int m_save_batch_flag;
+};
 
 extern struct cleanup *make_bpstat_clear_actions_cleanup (void);
 \f
-- 
2.9.5

[RFA 1/8] Change record_full_gdb_operation_disable_set not to return a cleanup

[Tom Tromey]

This changes record_full_gdb_operation_disable_set to return a
scoped_restore rather than a cleanup, and fixes all the users.

ChangeLog
2017-09-30  Tom Tromey  <tom@tromey.com>

	* record-full.h (record_full_gdb_operation_disable_set): Return
	scoped_restore_tmpl<int>.
	* infrun.c (adjust_pc_after_break): Update.
	(handle_signal_stop): Update.
	* record-full.c (record_full_gdb_operation_disable_set): Return
	scoped_restore_tmpl<int>.
	(record_full_wait_1, record_full_insert_breakpoint)
	(record_full_remove_breakpoint, record_full_save)
	(record_full_goto_insn): Update.
---
 gdb/ChangeLog     | 12 ++++++++++++
 gdb/infrun.c      | 15 +++++++--------
 gdb/record-full.c | 34 ++++++++++++----------------------
 gdb/record-full.h |  2 +-
 4 files changed, 32 insertions(+), 31 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index ed5d20c..a415c18 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,15 @@
+2017-09-30  Tom Tromey  <tom@tromey.com>
+
+	* record-full.h (record_full_gdb_operation_disable_set): Return
+	scoped_restore_tmpl<int>.
+	* infrun.c (adjust_pc_after_break): Update.
+	(handle_signal_stop): Update.
+	* record-full.c (record_full_gdb_operation_disable_set): Return
+	scoped_restore_tmpl<int>.
+	(record_full_wait_1, record_full_insert_breakpoint)
+	(record_full_remove_breakpoint, record_full_save)
+	(record_full_goto_insn): Update.
+
 2017-09-29  Tom Tromey  <tom@tromey.com>
 
 	* target.c (read_whatever_is_readable): Change type of "result".
diff --git a/gdb/infrun.c b/gdb/infrun.c
index fba0079..e82f61f 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -4178,10 +4178,11 @@ adjust_pc_after_break (struct thread_info *thread,
       || (target_is_non_stop_p ()
 	  && moribund_breakpoint_here_p (aspace, breakpoint_pc)))
     {
-      struct cleanup *old_cleanups = make_cleanup (null_cleanup, NULL);
+      gdb::optional<scoped_restore_tmpl<int>> restore_operation_disable;
 
       if (record_full_is_used ())
-	record_full_gdb_operation_disable_set ();
+	restore_operation_disable.emplace
+	  (record_full_gdb_operation_disable_set ());
 
       /* When using hardware single-step, a SIGTRAP is reported for both
 	 a completed single-step and a software breakpoint.  Need to
@@ -4205,8 +4206,6 @@ adjust_pc_after_break (struct thread_info *thread,
 	  || (thread->stepped_breakpoint
 	      && thread->prev_pc == breakpoint_pc))
 	regcache_write_pc (regcache, breakpoint_pc);
-
-      do_cleanups (old_cleanups);
     }
 }
 
@@ -6008,14 +6007,14 @@ handle_signal_stop (struct execution_control_state *ecs)
 	  decr_pc = gdbarch_decr_pc_after_break (gdbarch);
 	  if (decr_pc != 0)
 	    {
-	      struct cleanup *old_cleanups = make_cleanup (null_cleanup, NULL);
+	      gdb::optional<scoped_restore_tmpl<int>>
+		restore_operation_disable;
 
 	      if (record_full_is_used ())
-		record_full_gdb_operation_disable_set ();
+		restore_operation_disable.emplace
+		  (record_full_gdb_operation_disable_set ());
 
 	      regcache_write_pc (regcache, stop_pc + decr_pc);
-
-	      do_cleanups (old_cleanups);
 	    }
 	}
       else
diff --git a/gdb/record-full.c b/gdb/record-full.c
index 5073be3..5bd8900 100644
--- a/gdb/record-full.c
+++ b/gdb/record-full.c
@@ -670,16 +670,10 @@ record_full_message_wrapper_safe (struct regcache *regcache,
 
 static int record_full_gdb_operation_disable = 0;
 
-struct cleanup *
+scoped_restore_tmpl<int>
 record_full_gdb_operation_disable_set (void)
 {
-  struct cleanup *old_cleanups = NULL;
-
-  old_cleanups =
-    make_cleanup_restore_integer (&record_full_gdb_operation_disable);
-  record_full_gdb_operation_disable = 1;
-
-  return old_cleanups;
+  return make_scoped_restore (&record_full_gdb_operation_disable, 1);
 }
 
 /* Flag set to TRUE for target_stopped_by_watchpoint.  */
@@ -1051,7 +1045,8 @@ record_full_wait_1 (struct target_ops *ops,
 		    ptid_t ptid, struct target_waitstatus *status,
 		    int options)
 {
-  struct cleanup *set_cleanups = record_full_gdb_operation_disable_set ();
+  scoped_restore restore_operation_disable
+    = record_full_gdb_operation_disable_set ();
 
   if (record_debug)
     fprintf_unfiltered (gdb_stdlog,
@@ -1334,7 +1329,6 @@ replay_out:
 
   signal (SIGINT, handle_sigint);
 
-  do_cleanups (set_cleanups);
   return inferior_ptid;
 }
 
@@ -1653,12 +1647,11 @@ record_full_insert_breakpoint (struct target_ops *ops,
 	 However, we do have to insert software single-step
 	 breakpoints, in case the target can't hardware step.  To keep
 	 things simple, we always insert.  */
-      struct cleanup *old_cleanups;
       int ret;
 
-      old_cleanups = record_full_gdb_operation_disable_set ();
+      scoped_restore restore_operation_disable
+	= record_full_gdb_operation_disable_set ();
       ret = ops->beneath->to_insert_breakpoint (ops->beneath, gdbarch, bp_tgt);
-      do_cleanups (old_cleanups);
 
       if (ret != 0)
 	return ret;
@@ -1711,14 +1704,12 @@ record_full_remove_breakpoint (struct target_ops *ops,
 	{
 	  if (bp->in_target_beneath)
 	    {
-	      struct cleanup *old_cleanups;
 	      int ret;
 
-	      old_cleanups = record_full_gdb_operation_disable_set ();
+	      scoped_restore restore_operation_disable
+		= record_full_gdb_operation_disable_set ();
 	      ret = ops->beneath->to_remove_breakpoint (ops->beneath, gdbarch,
 							bp_tgt, reason);
-	      do_cleanups (old_cleanups);
-
 	      if (ret != 0)
 		return ret;
 	    }
@@ -2549,7 +2540,6 @@ record_full_save (struct target_ops *self, const char *recfilename)
   uint32_t magic;
   struct regcache *regcache;
   struct gdbarch *gdbarch;
-  struct cleanup *set_cleanups;
   int save_size = 0;
   asection *osec = NULL;
   int bfd_offset = 0;
@@ -2573,7 +2563,8 @@ record_full_save (struct target_ops *self, const char *recfilename)
   gdbarch = get_regcache_arch (regcache);
 
   /* Disable the GDB operation record.  */
-  set_cleanups = record_full_gdb_operation_disable_set ();
+  scoped_restore restore_operation_disable
+    = record_full_gdb_operation_disable_set ();
 
   /* Reverse execute to the begin of record list.  */
   while (1)
@@ -2737,7 +2728,6 @@ record_full_save (struct target_ops *self, const char *recfilename)
         record_full_list = record_full_list->prev;
     }
 
-  do_cleanups (set_cleanups);
   unlink_file.keep ();
 
   /* Succeeded.  */
@@ -2753,7 +2743,8 @@ static void
 record_full_goto_insn (struct record_full_entry *entry,
 		       enum exec_direction_kind dir)
 {
-  struct cleanup *set_cleanups = record_full_gdb_operation_disable_set ();
+  scoped_restore restore_operation_disable
+    = record_full_gdb_operation_disable_set ();
   struct regcache *regcache = get_current_regcache ();
   struct gdbarch *gdbarch = get_regcache_arch (regcache);
 
@@ -2771,7 +2762,6 @@ record_full_goto_insn (struct record_full_entry *entry,
       else
 	record_full_list = record_full_list->next;
     } while (record_full_list != entry);
-  do_cleanups (set_cleanups);
 }
 
 /* Alias for "target record-full".  */
diff --git a/gdb/record-full.h b/gdb/record-full.h
index 7d53b63..728bf59 100644
--- a/gdb/record-full.h
+++ b/gdb/record-full.h
@@ -29,6 +29,6 @@ extern int record_full_arch_list_add_end (void);
 /* Returns true if the process record target is open.  */
 extern int record_full_is_used (void);
 
-extern struct cleanup *record_full_gdb_operation_disable_set (void);
+extern scoped_restore_tmpl<int> record_full_gdb_operation_disable_set ();
 
 #endif /* RECORD_FULL_H */
-- 
2.9.5

[RFA 5/8] Use gdb::byte_vector in load_progress

[Tom Tromey]

This changes load_progress to use gdb::byte_vector, removing a
cleanup.

2017-09-30  Tom Tromey  <tom@tromey.com>

	* symfile.c (load_progress): Use gdb::byte_vector.
---
 gdb/ChangeLog | 4 ++++
 gdb/symfile.c | 9 ++++-----
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index ba841f8..f3faf73 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,9 @@
 2017-09-30  Tom Tromey  <tom@tromey.com>
 
+	* symfile.c (load_progress): Use gdb::byte_vector.
+
+2017-09-30  Tom Tromey  <tom@tromey.com>
+
 	* mi/mi-main.c (mi_cmd_trace_frame_collected): Remove unused
 	declaration.
 	* printcmd.c (x_command): Remove unused declaration.
diff --git a/gdb/symfile.c b/gdb/symfile.c
index f6bc378..a741654 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -56,6 +56,7 @@
 #include "stack.h"
 #include "gdb_bfd.h"
 #include "cli/cli-utils.h"
+#include "common/byte-vector.h"
 
 #include <sys/types.h>
 #include <fcntl.h>
@@ -1942,16 +1943,14 @@ load_progress (ULONGEST bytes, void *untyped_arg)
 	 might add a verify_memory() method to the target vector and
 	 then use that.  remote.c could implement that method using
 	 the ``qCRC'' packet.  */
-      gdb_byte *check = (gdb_byte *) xmalloc (bytes);
-      struct cleanup *verify_cleanups = make_cleanup (xfree, check);
+      gdb::byte_vector check (bytes);
 
-      if (target_read_memory (args->lma, check, bytes) != 0)
+      if (target_read_memory (args->lma, check.data (), bytes) != 0)
 	error (_("Download verify read failed at %s"),
 	       paddress (target_gdbarch (), args->lma));
-      if (memcmp (args->buffer, check, bytes) != 0)
+      if (memcmp (args->buffer, check.data (), bytes) != 0)
 	error (_("Download verify compare failed at %s"),
 	       paddress (target_gdbarch (), args->lma));
-      do_cleanups (verify_cleanups);
     }
   totals->data_count += bytes;
   args->lma += bytes;
-- 
2.9.5

Re: [RFA 3/8] Use std::string in utils.c

[Pedro Alves]

On 10/01/2017 05:06 AM, Tom Tromey wrote:

>    /* Don't allow infinite error/warning recursion.  */
>    {
> @@ -429,18 +428,17 @@ internal_vproblem (struct internal_problem *problem,
>      char *msg;
>  
>      msg = xstrvprintf (fmt, ap);
> -    reason = xstrprintf ("%s:%d: %s: %s\n"
> -			 "A problem internal to GDB has been detected,\n"
> -			 "further debugging may prove unreliable.",
> -			 file, line, problem->name, msg);
> +    reason = string_printf ("%s:%d: %s: %s\n"
> +			    "A problem internal to GDB has been detected,\n"
> +			    "further debugging may prove unreliable.",
> +			    file, line, problem->name, msg);
>      xfree (msg);
> -    make_cleanup (xfree, reason);

Could you please make msg use string_vprintf too while at it, thus
getting rid of that bare xfree?

(Similarly to the defaulted_query change below, though there we
were using a cleanup.)

Looks good to me with that change.

Thanks,
Pedro Alves

> @@ -978,13 +975,12 @@ defaulted_query (const char *ctlstr, const char defchar, va_list args)
>      }
>  
>    /* Format the question outside of the loop, to avoid reusing args.  */
> -  question = xstrvprintf (ctlstr, args);
> -  old_chain = make_cleanup (xfree, question);
> -  prompt = xstrprintf (_("%s%s(%s or %s) %s"),
> -		      annotation_level > 1 ? "\n\032\032pre-query\n" : "",
> -		      question, y_string, n_string,
> -		      annotation_level > 1 ? "\n\032\032query\n" : "");
> -  make_cleanup (xfree, prompt);
> +  std::string question = string_vprintf (ctlstr, args);
> +  std::string prompt
> +    = string_printf (_("%s%s(%s or %s) %s"),
> +		     annotation_level > 1 ? "\n\032\032pre-query\n" : "",
> +		     question.c_str (), y_string, n_string,
> +		     annotation_level > 1 ? "\n\032\032query\n" : "");
>  

Re: [RFA 0/8] more cleanup removal

[Pedro Alves]

On 10/01/2017 05:06 AM, Tom Tromey wrote:
> This series removes more cleanups from gdb.  I think each patch speaks
> for itself.  They should all be pretty straightforward.
> 
> Regression tested by the build bot.
> 
> After this, by the "grep 'struct cleanup'" metric, gdb has less than
> 500 remaining spots to fix.

Nice!

The series looks good to me.  If you could do the tweak
I mentioned in patch #3 I'd appreciate it.

Thanks,
Pedro Alves

Re: [RFA 3/8] Use std::string in utils.c

[Tom Tromey]

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

Pedro> Could you please make msg use string_vprintf too while at it, thus
Pedro> getting rid of that bare xfree?

Sure, I made this change.

Tom