This is the mail archive of the
gdb-cvs@sourceware.org
mailing list for the GDB project.
[binutils-gdb] C++-ify skip.c
- From: Tom Tromey <tromey at sourceware dot org>
- To: gdb-cvs at sourceware dot org
- Date: 9 Aug 2017 18:42:15 -0000
- Subject: [binutils-gdb] C++-ify skip.c
https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=42fa2e0e1b7b135ab30f2f78074c3bfffa04d0cc
commit 42fa2e0e1b7b135ab30f2f78074c3bfffa04d0cc
Author: Tom Tromey <tom@tromey.com>
Date: Sat Aug 5 16:40:56 2017 -0600
C++-ify skip.c
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-09 Tom Tromey <tom@tromey.com>
* skip.c (skiplist_entry): New constructor.
(skiplist_entry::enabled, skiplist_entry::function_is_regexp)
(skiplist_entry::file_is_glob): Now bool.
(skiplist_entry::file, skiplist_entry::function): Now
std::string.
(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)
(skip_form_bytes, compile_skip_regexp, skip_command, skip_info)
(skip_file_p, skip_gfile_p, skip_function_p, skip_rfunction_p)
(function_name_is_marked_for_skip): Update.
(skip_delete_command): Update. Use delete.
Diff:
---
gdb/ChangeLog | 16 ++++++
gdb/skip.c | 171 +++++++++++++++++++++++++++-------------------------------
2 files changed, 95 insertions(+), 92 deletions(-)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 4e30810..55a0b12 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,19 @@
+2017-08-09 Tom Tromey <tom@tromey.com>
+
+ * skip.c (skiplist_entry): New constructor.
+ (skiplist_entry::enabled, skiplist_entry::function_is_regexp)
+ (skiplist_entry::file_is_glob): Now bool.
+ (skiplist_entry::file, skiplist_entry::function): Now
+ std::string.
+ (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)
+ (skip_form_bytes, compile_skip_regexp, skip_command, skip_info)
+ (skip_file_p, skip_gfile_p, skip_function_p, skip_rfunction_p)
+ (function_name_is_marked_for_skip): Update.
+ (skip_delete_command): Update. Use delete.
+
2017-08-09 Jiong Wang <jiong.wang@arm.com>
* aarch64-linux-tdep.c: Include "auxv.h" and "elf/common.h".
diff --git a/gdb/skip.c b/gdb/skip.c
index bf44913..6ab6c91 100644
--- a/gdb/skip.c
+++ b/gdb/skip.c
@@ -38,34 +38,44 @@
struct skiplist_entry
{
+ skiplist_entry (bool file_is_glob_, std::string &&file_,
+ bool function_is_regexp_, std::string &&function_)
+ : number (-1),
+ file_is_glob (file_is_glob_),
+ file (std::move (file_)),
+ function_is_regexp (function_is_regexp_),
+ function (std::move (function_)),
+ enabled (true),
+ next (NULL)
+ {
+ }
+
int number;
- /* Non-zero if FILE is a glob-style pattern.
- Otherewise it is the plain file name (possibly with directories). */
- int file_is_glob;
+ /* True if FILE is a glob-style pattern.
+ Otherwise it is the plain file name (possibly with directories). */
+ bool file_is_glob;
- /* The name of the file or NULL.
- The skiplist entry owns this pointer. */
- char *file;
+ /* The name of the file or empty if no name. */
+ std::string file;
- /* Non-zero if FUNCTION is a regexp.
+ /* True if FUNCTION is a regexp.
Otherwise it is a plain function name (possibly with arguments,
for C++). */
- int function_is_regexp;
+ bool function_is_regexp;
- /* The name of the function or NULL.
- The skiplist entry owns this pointer. */
- char *function;
+ /* The name of the function or empty if no name. */
+ std::string function;
/* 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 +90,19 @@ static int skiplist_entry_count;
/* Create a skip object. */
-static struct skiplist_entry *
-make_skip_entry (int file_is_glob, const char *file,
- int function_is_regexp, const char *function)
+static std::unique_ptr<skiplist_entry>
+make_skip_entry (bool file_is_glob, std::string &&file,
+ bool function_is_regexp, std::string &&function)
{
- struct skiplist_entry *e = XCNEW (struct skiplist_entry);
-
- gdb_assert (file != NULL || function != NULL);
+ gdb_assert (!file.empty () || !function.empty ());
if (file_is_glob)
- gdb_assert (file != NULL);
+ gdb_assert (!file.empty ());
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. */
+ gdb_assert (!function.empty ());
-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, std::move (file),
+ function_is_regexp, std::move (function)));
}
static void
@@ -150,7 +126,8 @@ skip_file_command (char *arg, int from_tty)
else
filename = arg;
- add_skiplist_entry (make_skip_entry (0, filename, 0, NULL));
+ add_skiplist_entry (make_skip_entry (false, std::string (filename), false,
+ std::string ()));
printf_filtered (_("File %s will be skipped when stepping.\n"), filename);
}
@@ -161,7 +138,8 @@ skip_file_command (char *arg, int from_tty)
static void
skip_function (const char *name)
{
- add_skiplist_entry (make_skip_entry (0, NULL, 0, name));
+ add_skiplist_entry (make_skip_entry (false, std::string (), false,
+ std::string (name)));
printf_filtered (_("Function %s will be skipped when stepping.\n"), name);
}
@@ -204,8 +182,8 @@ compile_skip_regexp (struct skiplist_entry *e, const char *message)
flags |= REG_EXTENDED;
#endif
- gdb_assert (e->function_is_regexp && e->function != NULL);
- e->compiled_function_regexp.emplace (e->function, flags, message);
+ gdb_assert (e->function_is_regexp && !e->function.empty ());
+ e->compiled_function_regexp.emplace (e->function.c_str (), flags, message);
}
/* Process "skip ..." that does not match "skip file" or "skip function". */
@@ -217,7 +195,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 +268,23 @@ 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::string entry_file;
+ if (file != NULL)
+ entry_file = file;
+ else if (gfile != NULL)
+ entry_file = gfile;
+ std::string entry_function;
+ if (function != NULL)
+ entry_function = function;
+ else if (rfunction != NULL)
+ entry_function = rfunction;
+ std::unique_ptr<skiplist_entry> e
+ = make_skip_entry (gfile != NULL, std::move (entry_file),
+ rfunction != NULL, std::move (entry_function));
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
@@ -392,14 +376,16 @@ skip_info (char *arg, int from_tty)
current_uiout->field_string ("regexp", "n"); /* 3 */
current_uiout->field_string ("file",
- e->file ? e->file : "<none>"); /* 4 */
+ e->file.empty () ? "<none>"
+ : e->file.c_str ()); /* 4 */
if (e->function_is_regexp)
current_uiout->field_string ("regexp", "y"); /* 5 */
else
current_uiout->field_string ("regexp", "n"); /* 5 */
- current_uiout->field_string (
- "function", e->function ? e->function : "<none>"); /* 6 */
+ current_uiout->field_string ("function",
+ e->function.empty () ? "<none>"
+ : e->function.c_str ()); /* 6 */
current_uiout->text ("\n");
}
@@ -414,7 +400,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 +417,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 +440,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 +455,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 +466,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 ();
}
}
@@ -495,28 +481,29 @@ static int
skip_file_p (struct skiplist_entry *e,
const struct symtab_and_line *function_sal)
{
- gdb_assert (e->file != NULL && !e->file_is_glob);
+ gdb_assert (!e->file.empty () && !e->file_is_glob);
if (function_sal->symtab == NULL)
return 0;
/* Check first sole SYMTAB->FILENAME. It may not be a substring of
symtab_to_fullname as it may contain "./" etc. */
- if (compare_filenames_for_search (function_sal->symtab->filename, e->file))
+ if (compare_filenames_for_search (function_sal->symtab->filename,
+ e->file.c_str ()))
return 1;
/* Before we invoke realpath, which can get expensive when many
files are involved, do a quick comparison of the basenames. */
if (!basenames_may_differ
&& filename_cmp (lbasename (function_sal->symtab->filename),
- lbasename (e->file)) != 0)
+ lbasename (e->file.c_str ())) != 0)
return 0;
/* Note: symtab_to_fullname caches its result, thus we don't have to. */
{
const char *fullname = symtab_to_fullname (function_sal->symtab);
- if (compare_filenames_for_search (fullname, e->file))
+ if (compare_filenames_for_search (fullname, e->file.c_str ()))
return 1;
}
@@ -529,14 +516,14 @@ static int
skip_gfile_p (struct skiplist_entry *e,
const struct symtab_and_line *function_sal)
{
- gdb_assert (e->file != NULL && e->file_is_glob);
+ gdb_assert (!e->file.empty () && e->file_is_glob);
if (function_sal->symtab == NULL)
return 0;
/* Check first sole SYMTAB->FILENAME. It may not be a substring of
symtab_to_fullname as it may contain "./" etc. */
- if (gdb_filename_fnmatch (e->file, function_sal->symtab->filename,
+ if (gdb_filename_fnmatch (e->file.c_str (), function_sal->symtab->filename,
FNM_FILE_NAME | FNM_NOESCAPE) == 0)
return 1;
@@ -546,7 +533,7 @@ skip_gfile_p (struct skiplist_entry *e,
If the basename of the glob pattern is something like "*.c" then this
isn't much of a win. Oh well. */
if (!basenames_may_differ
- && gdb_filename_fnmatch (lbasename (e->file),
+ && gdb_filename_fnmatch (lbasename (e->file.c_str ()),
lbasename (function_sal->symtab->filename),
FNM_FILE_NAME | FNM_NOESCAPE) != 0)
return 0;
@@ -555,7 +542,7 @@ skip_gfile_p (struct skiplist_entry *e,
{
const char *fullname = symtab_to_fullname (function_sal->symtab);
- if (compare_glob_filenames_for_search (fullname, e->file))
+ if (compare_glob_filenames_for_search (fullname, e->file.c_str ()))
return 1;
}
@@ -567,8 +554,8 @@ skip_gfile_p (struct skiplist_entry *e,
static int
skip_function_p (struct skiplist_entry *e, const char *function_name)
{
- gdb_assert (e->function != NULL && !e->function_is_regexp);
- return strcmp_iw (function_name, e->function) == 0;
+ gdb_assert (!e->function.empty () && !e->function_is_regexp);
+ return strcmp_iw (function_name, e->function.c_str ()) == 0;
}
/* Return non-zero if we're stopped at a function regexp to be skipped. */
@@ -576,7 +563,7 @@ skip_function_p (struct skiplist_entry *e, const char *function_name)
static int
skip_rfunction_p (struct skiplist_entry *e, const char *function_name)
{
- gdb_assert (e->function != NULL && e->function_is_regexp
+ gdb_assert (!e->function.empty () && e->function_is_regexp
&& e->compiled_function_regexp);
return (e->compiled_function_regexp->exec (function_name, 0, NULL, 0)
== 0);
@@ -601,7 +588,7 @@ function_name_is_marked_for_skip (const char *function_name,
if (!e->enabled)
continue;
- if (e->file != NULL)
+ if (!e->file.empty ())
{
if (e->file_is_glob)
{
@@ -614,7 +601,7 @@ function_name_is_marked_for_skip (const char *function_name,
skip_by_file = 1;
}
}
- if (e->function != NULL)
+ if (!e->function.empty ())
{
if (e->function_is_regexp)
{
@@ -630,7 +617,7 @@ function_name_is_marked_for_skip (const char *function_name,
/* If both file and function must match, make sure we don't errantly
exit if only one of them match. */
- if (e->file != NULL && e->function != NULL)
+ if (!e->file.empty () && !e->function.empty ())
{
if (skip_by_file && skip_by_function)
return 1;