[RFA] C++-ify skip.c

[Tom Tromey <tom at tromey dot com>]

I happened to notice that skiplist_entry, in skip.c, contains a
gdb::optional<compiled_regex> -- but that this object's destructor is
never run.  This can result in a memory leak.

This patch fixes the bug by applying a bit more C++: changing this
code to use new and delete, and std::unique_ptr; and removing cleanups
in the process.

Built and regression tested on x86-64 Fedora 25.

ChangeLog
2017-08-06  Tom Tromey  <tom@tromey.com>

	* skip.c (skiplist_entry): New constructor.
	(~skiplist_entry): New destructor.
	(skiplist_entry::enabled): Now bool.
	(make_skip_entry): Return a unique_ptr.  Use new.
	(free_skiplist_entry, free_skiplist_entry_cleanup)
	(make_free_skiplist_entry_cleanup): Remove.
	(skip_command, skip_disable_command, add_skiplist_entry): Update.
	(skip_delete_command): Update.  Use delete.
---
 gdb/ChangeLog | 11 ++++++++
 gdb/skip.c    | 89 +++++++++++++++++++++++------------------------------------
 2 files changed, 45 insertions(+), 55 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 722fade..12e0d02 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,14 @@
+2017-08-06  Tom Tromey  <tom@tromey.com>
+
+	* skip.c (skiplist_entry): New constructor.
+	(~skiplist_entry): New destructor.
+	(skiplist_entry::enabled): Now bool.
+	(make_skip_entry): Return a unique_ptr.  Use new.
+	(free_skiplist_entry, free_skiplist_entry_cleanup)
+	(make_free_skiplist_entry_cleanup): Remove.
+	(skip_command, skip_disable_command, add_skiplist_entry): Update.
+	(skip_delete_command): Update.  Use delete.
+
 2017-08-05  Tom Tromey  <tom@tromey.com>
 
 	* compile/compile-object-load.c (compile_object_load): Use
diff --git a/gdb/skip.c b/gdb/skip.c
index bf44913..f3291f3 100644
--- a/gdb/skip.c
+++ b/gdb/skip.c
@@ -38,6 +38,24 @@
 
 struct skiplist_entry
 {
+  skiplist_entry (int file_is_glob_, const char *file_,
+		  int function_is_regexp_, const char *function_)
+    : number (-1),
+      file_is_glob (file_is_glob_),
+      file (file_ == NULL ? NULL : xstrdup (file_)),
+      function_is_regexp (function_is_regexp_),
+      function (function_ == NULL ? NULL : xstrdup (function_)),
+      enabled (true),
+      next (NULL)
+  {
+  }
+
+  ~skiplist_entry ()
+  {
+    xfree (file);
+    xfree (function);
+  }
+
   int number;
 
   /* Non-zero if FILE is a glob-style pattern.
@@ -60,12 +78,12 @@ struct skiplist_entry
   /* If this is a function regexp, the compiled form.  */
   gdb::optional<compiled_regex> compiled_function_regexp;
 
-  int enabled;
+  bool enabled;
 
   struct skiplist_entry *next;
 };
 
-static void add_skiplist_entry (struct skiplist_entry *e);
+static void add_skiplist_entry (std::unique_ptr<skiplist_entry> &&e);
 
 static struct skiplist_entry *skiplist_entry_chain;
 static int skiplist_entry_count;
@@ -80,53 +98,18 @@ static int skiplist_entry_count;
 
 /* Create a skip object.  */
 
-static struct skiplist_entry *
+static std::unique_ptr<skiplist_entry>
 make_skip_entry (int file_is_glob, const char *file,
 		 int function_is_regexp, const char *function)
 {
-  struct skiplist_entry *e = XCNEW (struct skiplist_entry);
-
   gdb_assert (file != NULL || function != NULL);
   if (file_is_glob)
     gdb_assert (file != NULL);
   if (function_is_regexp)
     gdb_assert (function != NULL);
 
-  if (file != NULL)
-    e->file = xstrdup (file);
-  if (function != NULL)
-    e->function = xstrdup (function);
-  e->file_is_glob = file_is_glob;
-  e->function_is_regexp = function_is_regexp;
-  e->enabled = 1;
-
-  return e;
-}
-
-/* Free a skiplist entry.  */
-
-static void
-free_skiplist_entry (struct skiplist_entry *e)
-{
-  xfree (e->file);
-  xfree (e->function);
-  xfree (e);
-}
-
-/* Wrapper to free_skiplist_entry for use as a cleanup.  */
-
-static void
-free_skiplist_entry_cleanup (void *e)
-{
-  free_skiplist_entry ((struct skiplist_entry *) e);
-}
-
-/* Create a cleanup to free skiplist entry E.  */
-
-static struct cleanup *
-make_free_skiplist_entry_cleanup (struct skiplist_entry *e)
-{
-  return make_cleanup (free_skiplist_entry_cleanup, e);
+  return std::unique_ptr<skiplist_entry>
+    (new skiplist_entry (file_is_glob, file, function_is_regexp, function));
 }
 
 static void
@@ -217,7 +200,6 @@ skip_command (char *arg, int from_tty)
   const char *gfile = NULL;
   const char *function = NULL;
   const char *rfunction = NULL;
-  struct skiplist_entry *e;
   int i;
 
   if (arg == NULL)
@@ -291,16 +273,13 @@ skip_command (char *arg, int from_tty)
   gdb_assert (file != NULL || gfile != NULL
 	      || function != NULL || rfunction != NULL);
 
-  e = make_skip_entry (gfile != NULL, file ? file : gfile,
-		       rfunction != NULL, function ? function : rfunction);
+  std::unique_ptr<skiplist_entry> e
+    = make_skip_entry (gfile != NULL, file ? file : gfile, rfunction != NULL,
+		       function ? function : rfunction);
   if (rfunction != NULL)
-    {
-      struct cleanup *rf_cleanups = make_free_skiplist_entry_cleanup (e);
+    compile_skip_regexp (e.get (), _("regexp"));
 
-      compile_skip_regexp (e, _("regexp"));
-      discard_cleanups (rf_cleanups);
-    }
-  add_skiplist_entry (e);
+  add_skiplist_entry (std::move (e));
 
   /* I18N concerns drive some of the choices here (we can't piece together
      the output too much).  OTOH we want to keep this simple.  Therefore the
@@ -414,7 +393,7 @@ skip_enable_command (char *arg, int from_tty)
   ALL_SKIPLIST_ENTRIES (e)
     if (arg == NULL || number_is_in_list (arg, e->number))
       {
-        e->enabled = 1;
+        e->enabled = true;
         found = 1;
       }
 
@@ -431,7 +410,7 @@ skip_disable_command (char *arg, int from_tty)
   ALL_SKIPLIST_ENTRIES (e)
     if (arg == NULL || number_is_in_list (arg, e->number))
       {
-	e->enabled = 0;
+	e->enabled = false;
         found = 1;
       }
 
@@ -454,7 +433,7 @@ skip_delete_command (char *arg, int from_tty)
 	else
 	  skiplist_entry_chain = e->next;
 
-	free_skiplist_entry (e);
+	delete e;
         found = 1;
       }
     else
@@ -469,7 +448,7 @@ skip_delete_command (char *arg, int from_tty)
 /* Add the given skiplist entry to our list, and set the entry's number.  */
 
 static void
-add_skiplist_entry (struct skiplist_entry *e)
+add_skiplist_entry (std::unique_ptr<skiplist_entry> &&e)
 {
   struct skiplist_entry *e1;
 
@@ -480,12 +459,12 @@ add_skiplist_entry (struct skiplist_entry *e)
 
   e1 = skiplist_entry_chain;
   if (e1 == NULL)
-    skiplist_entry_chain = e;
+    skiplist_entry_chain = e.release ();
   else
     {
       while (e1->next)
 	e1 = e1->next;
-      e1->next = e;
+      e1->next = e.release ();
     }
 }
 
-- 
2.9.4