Re: [PATCH] More gdb/skip.c C++ification

Pedro Alves
2017-08-11T11:23:26Z

On 08/10/2017 10:10 PM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
>
> Pedro> It's a bit of churn, but there's nothing really complicated here.
> Pedro> It's just the next logical steps after Tom's patch. See commit
> Pedro> log below.
>
> Pedro> WDYT?
>
> It all seems good to me (though I only skimmed it).
>

Thanks, I pushed it in with the std::list<T*> -> std::list<T>
change. Only downside is that I had to make the ctor public again (with
the private class key idiom), so that I could use std::list::emplace_back.
If we ever need to make skiplist_entry polymorphic, or need to worry about
performance, we could switch back to unique_ptr, or likely better use a
std::vector of pointers, for better locality / memory access patterns.

From de7985c3cca1358b21b49a9872455e2032f48ee3 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Fri, 11 Aug 2017 12:11:28 +0100
Subject: [PATCH] More gdb/skip.c C++ification

- Make skiplist_entry a class with private data members.
- Move all construction logic to the ctor.
- Make skip_file_p etc be methods of skiplist_entry.
- Use std::list for the skip entries chain. Make the list own its
elements.
- Get rid of the ALL_SKIPLIST_ENTRIES/ALL_SKIPLIST_ENTRIES_SAFE
macros, use range-for / iterators instead.
- function_name_is_marked_for_skip 'function_sal' argument must be
non-NULL, so make it a reference instead.

All skiplist_entry invariants are now controlled by skiplist_entry
methods/internals. Some gdb_asserts disappear for being redundant.

gdb/ChangeLog:
2017-08-11 Pedro Alves <palves@redhat.com>

* infrun.c (process_event_stop_test): Adjust
function_name_is_marked_for_skip call.
* skip.c: Include .
(skiplist_entry): Make it a class with private fields, and
getters/setters.
(skiplist_entry_chain): Delete.
(skiplist_entries): New.
(skiplist_entry_count): Delete.
(highest_skiplist_entry_num): New.
(ALL_SKIPLIST_ENTRIES, ALL_SKIPLIST_ENTRIES_SAFE): Delete.
(add_skiplist_entry): Delete.
(skiplist_entry::skiplist_entry): New.
(skiplist_entry::add_entry): New.
(skip_file_command, skip_function): Adjust.
(compile_skip_regexp): Delete.
(skip_command): Don't compile regexp here. Adjust to use
skiplist_entry::add_entry.
(skip_info): Adjust to use range-for and getters.
(skip_enable_command, skip_disable_command): Adjust to use
range-for and setters.
(skip_delete_command): Adjust to use std::list.
(add_skiplist_entry): Delete.
(skip_file_p): Delete, refactored as ...
(skiplist_entry::do_skip_file_p): ... this new method.
(skip_gfile_p): Delete, refactored as ...
(skiplist_entry::do_gskip_file_p): ... this new method.
(skip_function_p, skip_rfunction_p): Delete, refactored as ...
(skiplist_entry::skip_function_p): ... this new method.
(function_name_is_marked_for_skip): Now returns bool, and takes
the function sal by const reference. Adjust to use range-for and
skiplist_entry methods.
(_initialize_step_skip): Remove references to
skiplist_entry_chain, skiplist_entry_count.
* skip.h (function_name_is_marked_for_skip): Now returns bool, and
takes the function sal by const reference.
---
gdb/ChangeLog | 38 ++++++
gdb/infrun.c | 2 +-
gdb/skip.c | 433 +++++++++++++++++++++++++++-------------------------------
gdb/skip.h | 8 +-
4 files changed, 243 insertions(+), 238 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 75a1ea8..c588291 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,41 @@
+2017-08-11 Pedro Alves <palves@redhat.com>
+
+ * infrun.c (process_event_stop_test): Adjust
+ function_name_is_marked_for_skip call.
+ * skip.c: Include .
+ (skiplist_entry): Make it a class with private fields, and
+ getters/setters.
+ (skiplist_entry_chain): Delete.
+ (skiplist_entries): New.
+ (skiplist_entry_count): Delete.
+ (highest_skiplist_entry_num): New.
+ (ALL_SKIPLIST_ENTRIES, ALL_SKIPLIST_ENTRIES_SAFE): Delete.
+ (add_skiplist_entry): Delete.
+ (skiplist_entry::skiplist_entry): New.
+ (skiplist_entry::add_entry): New.
+ (skip_file_command, skip_function): Adjust.
+ (compile_skip_regexp): Delete.
+ (skip_command): Don't compile regexp here. Adjust to use
+ skiplist_entry::add_entry.
+ (skip_info): Adjust to use range-for and getters.
+ (skip_enable_command, skip_disable_command): Adjust to use
+ range-for and setters.
+ (skip_delete_command): Adjust to use std::list.
+ (add_skiplist_entry): Delete.
+ (skip_file_p): Delete, refactored as ...
+ (skiplist_entry::do_skip_file_p): ... this new method.
+ (skip_gfile_p): Delete, refactored as ...
+ (skiplist_entry::do_gskip_file_p): ... this new method.
+ (skip_function_p, skip_rfunction_p): Delete, refactored as ...
+ (skiplist_entry::skip_function_p): ... this new method.
+ (function_name_is_marked_for_skip): Now returns bool, and takes
+ the function sal by const reference. Adjust to use range-for and
+ skiplist_entry methods.
+ (_initialize_step_skip): Remove references to
+ skiplist_entry_chain, skiplist_entry_count.
+ * skip.h (function_name_is_marked_for_skip): Now returns bool, and
+ takes the function sal by const reference.
+
2017-08-11 Yao Qi <yao.qi@linaro.org>

* dwarf2-frame.c (clear_pointer_cleanup): Remove.
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 8f966e2..d6723fd 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -6788,7 +6788,7 @@ process_event_stop_test (struct execution_control_state *ecs)
tmp_sal = find_pc_line (ecs->stop_func_start, 0);
if (tmp_sal.line != 0
&& !function_name_is_marked_for_skip (ecs->stop_func_name,
- &tmp_sal))
+ tmp_sal))
{
if (execution_direction == EXEC_REVERSE)
handle_step_into_function_backward (gdbarch, ecs);
diff --git a/gdb/skip.c b/gdb/skip.c
index 6ab6c91..9f77248 100644
--- a/gdb/skip.c
+++ b/gdb/skip.c
@@ -35,74 +35,129 @@
#include "fnmatch.h"
#include "gdb_regex.h"
#include "common/gdb_optional.h"
+#include

-struct skiplist_entry
+class 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;
+public:
+ /* Create a skiplist_entry object and add it to the chain. */
+ static void add_entry (bool file_is_glob,
+ std::string &&file,
+ bool function_is_regexp,
+ std::string &&function);
+
+ /* Return true if the skip entry has a file or glob-style file
+ pattern that matches FUNCTION_SAL. */
+ bool skip_file_p (const symtab_and_line &function_sal) const;
+
+ /* Return true if the skip entry has a function or function regexp
+ that matches FUNCTION_NAME. */
+ bool skip_function_p (const char *function_name) const;
+
+ /* Getters. */
+ int number () const { return m_number; };
+ bool enabled () const { return m_enabled; };
+ bool file_is_glob () const { return m_file_is_glob; }
+ const std::string &file () const { return m_file; }
+ const std::string &function () const { return m_function; }
+ bool function_is_regexp () const { return m_function_is_regexp; }
+
+ /* Setters. */
+ void enable () { m_enabled = true; };
+ void disable () { m_enabled = false; };
+
+ /* Disable copy. */
+ skiplist_entry (const skiplist_entry &) = delete;
+ void operator= (const skiplist_entry &) = delete;
+
+private:
+ /* Key that grants access to the constructor. */
+ struct private_key {};
+public:
+ /* Public so we can construct with container::emplace_back. Since
+ it requires a private class key, it can't be called from outside.
+ Use the add_entry static factory method to construct instead. */
+ skiplist_entry (bool file_is_glob, std::string &&file,
+ bool function_is_regexp, std::string &&function,
+ private_key);
+
+private:
+ /* Return true if we're stopped at a file to be skipped. */
+ bool do_skip_file_p (const symtab_and_line &function_sal) const;
+
+ /* Return true if we're stopped at a globbed file to be skipped. */
+ bool do_skip_gfile_p (const symtab_and_line &function_sal) const;
+
+private: /* data */
+ int m_number = -1;

/* True if FILE is a glob-style pattern.
Otherwise it is the plain file name (possibly with directories). */
- bool file_is_glob;
+ bool m_file_is_glob;

/* The name of the file or empty if no name. */
- std::string file;
+ std::string m_file;

/* True if FUNCTION is a regexp.
Otherwise it is a plain function name (possibly with arguments,
for C++). */
- bool function_is_regexp;
+ bool m_function_is_regexp;

/* The name of the function or empty if no name. */
- std::string function;
+ std::string m_function;

/* If this is a function regexp, the compiled form. */
- gdb::optional<compiled_regex> compiled_function_regexp;
+ gdb::optional<compiled_regex> m_compiled_function_regexp;

- bool enabled;
-
- struct skiplist_entry *next;
+ /* Enabled/disabled state. */
+ bool m_enabled = true;
};

-static void add_skiplist_entry (std::unique_ptr<skiplist_entry> &&e);
+static std::list<skiplist_entry> skiplist_entries;
+static int highest_skiplist_entry_num = 0;
+
+skiplist_entry::skiplist_entry (bool file_is_glob,
+ std::string &&file,
+ bool function_is_regexp,
+ std::string &&function,
+ private_key)
+ : m_file_is_glob (file_is_glob),
+ m_file (std::move (file)),
+ m_function_is_regexp (function_is_regexp),
+ m_function (std::move (function))
+{
+ gdb_assert (!m_file.empty () || !m_function.empty ());

-static struct skiplist_entry *skiplist_entry_chain;
-static int skiplist_entry_count;
+ if (m_file_is_glob)
+ gdb_assert (!m_file.empty ());

-#define ALL_SKIPLIST_ENTRIES(E) \
- for (E = skiplist_entry_chain; E; E = E->next)
+ if (m_function_is_regexp)
+ {
+ gdb_assert (!m_function.empty ());

-#define ALL_SKIPLIST_ENTRIES_SAFE(E,TMP) \
- for (E = skiplist_entry_chain; \
- E ? (TMP = E->next, 1) : 0; \
- E = TMP)
+ int flags = REG_NOSUB;
+#ifdef REG_EXTENDED
+ flags |= REG_EXTENDED;
+#endif

-/* Create a skip object. */
+ gdb_assert (!m_function.empty ());
+ m_compiled_function_regexp.emplace (m_function.c_str (), flags,
+ _("regexp"));
+ }
+}

-static std::unique_ptr<skiplist_entry>
-make_skip_entry (bool file_is_glob, std::string &&file,
- bool function_is_regexp, std::string &&function)
+void
+skiplist_entry::add_entry (bool file_is_glob, std::string &&file,
+ bool function_is_regexp, std::string &&function)
{
- gdb_assert (!file.empty () || !function.empty ());
- if (file_is_glob)
- gdb_assert (!file.empty ());
- if (function_is_regexp)
- gdb_assert (!function.empty ());
-
- return std::unique_ptr<skiplist_entry>
- (new skiplist_entry (file_is_glob, std::move (file),
- function_is_regexp, std::move (function)));
+ skiplist_entries.emplace_back (file_is_glob,
+ std::move (file),
+ function_is_regexp,
+ std::move (function),
+ private_key {});
+
+ /* Incremented after push_back, in case push_back throws. */
+ skiplist_entries.back ().m_number = ++highest_skiplist_entry_num;
}

static void
@@ -126,8 +181,8 @@ skip_file_command (char *arg, int from_tty)
else
filename = arg;

- add_skiplist_entry (make_skip_entry (false, std::string (filename), false,
- std::string ()));
+ skiplist_entry::add_entry (false, std::string (filename),
+ false, std::string ());

printf_filtered (_("File %s will be skipped when stepping.\n"), filename);
}
@@ -138,8 +193,7 @@ skip_file_command (char *arg, int from_tty)
static void
skip_function (const char *name)
{
- add_skiplist_entry (make_skip_entry (false, std::string (), false,
- std::string (name)));
+ skiplist_entry::add_entry (false, std::string (), false, std::string (name));

printf_filtered (_("Function %s will be skipped when stepping.\n"), name);
}
@@ -169,23 +223,6 @@ skip_function_command (char *arg, int from_tty)
skip_function (arg);
}

-/* Compile the regexp in E.
- An error is thrown if there's an error.
- MESSAGE is used as a prefix of the error message. */
-
-static void
-compile_skip_regexp (struct skiplist_entry *e, const char *message)
-{
- int flags = REG_NOSUB;
-
-#ifdef REG_EXTENDED
- flags |= REG_EXTENDED;
-#endif
-
- 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". */

static void
@@ -273,18 +310,15 @@ skip_command (char *arg, int from_tty)
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)
- compile_skip_regexp (e.get (), _("regexp"));

- add_skiplist_entry (std::move (e));
+ skiplist_entry::add_entry (gfile != NULL, std::move (entry_file),
+ rfunction != NULL, std::move (entry_function));

/* 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
@@ -321,7 +355,6 @@ skip_command (char *arg, int from_tty)
static void
skip_info (char *arg, int from_tty)
{
- struct skiplist_entry *e;
int num_printable_entries = 0;
struct value_print_options opts;

@@ -329,8 +362,8 @@ skip_info (char *arg, int from_tty)

/* Count the number of rows in the table and see if we need space for a
64-bit address anywhere. */
- ALL_SKIPLIST_ENTRIES (e)
- if (arg == NULL || number_is_in_list (arg, e->number))
+ for (const skiplist_entry &e : skiplist_entries)
+ if (arg == NULL || number_is_in_list (arg, e.number ()))
num_printable_entries++;

if (num_printable_entries == 0)
@@ -355,37 +388,36 @@ skip_info (char *arg, int from_tty)
current_uiout->table_header (40, ui_noalign, "function", "Function"); /* 6 */
current_uiout->table_body ();

- ALL_SKIPLIST_ENTRIES (e)
+ for (const skiplist_entry &e : skiplist_entries)
{
-
QUIT;
- if (arg != NULL && !number_is_in_list (arg, e->number))
+ if (arg != NULL && !number_is_in_list (arg, e.number ()))
continue;

ui_out_emit_tuple tuple_emitter (current_uiout, "blklst-entry");
- current_uiout->field_int ("number", e->number); /* 1 */
+ current_uiout->field_int ("number", e.number ()); /* 1 */

- if (e->enabled)
+ if (e.enabled ())
current_uiout->field_string ("enabled", "y"); /* 2 */
else
current_uiout->field_string ("enabled", "n"); /* 2 */

- if (e->file_is_glob)
+ if (e.file_is_glob ())
current_uiout->field_string ("regexp", "y"); /* 3 */
else
current_uiout->field_string ("regexp", "n"); /* 3 */

current_uiout->field_string ("file",
- e->file.empty () ? "<none>"
- : e->file.c_str ()); /* 4 */
- if (e->function_is_regexp)
+ 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.empty () ? "<none>"
- : e->function.c_str ()); /* 6 */
+ e.function ().empty () ? "<none>"
+ : e.function ().c_str ()); /* 6 */

current_uiout->text ("\n");
}
@@ -394,14 +426,13 @@ skip_info (char *arg, int from_tty)
static void
skip_enable_command (char *arg, int from_tty)
{
- struct skiplist_entry *e;
- int found = 0;
+ bool found = false;

- ALL_SKIPLIST_ENTRIES (e)
- if (arg == NULL || number_is_in_list (arg, e->number))
+ for (skiplist_entry &e : skiplist_entries)
+ if (arg == NULL || number_is_in_list (arg, e.number ()))
{
- e->enabled = true;
- found = 1;
+ e.enable ();
+ found = true;
}

if (!found)
@@ -411,14 +442,13 @@ skip_enable_command (char *arg, int from_tty)
static void
skip_disable_command (char *arg, int from_tty)
{
- struct skiplist_entry *e;
- int found = 0;
+ bool found = false;

- ALL_SKIPLIST_ENTRIES (e)
- if (arg == NULL || number_is_in_list (arg, e->number))
+ for (skiplist_entry &e : skiplist_entries)
+ if (arg == NULL || number_is_in_list (arg, e.number ()))
{
- e->enabled = false;
- found = 1;
+ e.disable ();
+ found = true;
}

if (!found)
@@ -428,104 +458,62 @@ skip_disable_command (char *arg, int from_tty)
static void
skip_delete_command (char *arg, int from_tty)
{
- struct skiplist_entry *e, *temp, *b_prev;
- int found = 0;
+ bool found = false;

- b_prev = 0;
- ALL_SKIPLIST_ENTRIES_SAFE (e, temp)
- if (arg == NULL || number_is_in_list (arg, e->number))
- {
- if (b_prev != NULL)
- b_prev->next = e->next;
- else
- skiplist_entry_chain = e->next;
+ for (auto it = skiplist_entries.begin (),
+ end = skiplist_entries.end ();
+ it != end;)
+ {
+ const skiplist_entry &e = *it;

- delete e;
- found = 1;
- }
- else
- {
- b_prev = e;
- }
+ if (arg == NULL || number_is_in_list (arg, e.number ()))
+ {
+ it = skiplist_entries.erase (it);
+ found = true;
+ }
+ else
+ ++it;
+ }

if (!found)
error (_("No skiplist entries found with number %s."), arg);
}

-/* Add the given skiplist entry to our list, and set the entry's number. */
-
-static void
-add_skiplist_entry (std::unique_ptr<skiplist_entry> &&e)
-{
- struct skiplist_entry *e1;
-
- e->number = ++skiplist_entry_count;
-
- /* Add to the end of the chain so that the list of
- skiplist entries will be in numerical order. */
-
- e1 = skiplist_entry_chain;
- if (e1 == NULL)
- skiplist_entry_chain = e.release ();
- else
- {
- while (e1->next)
- e1 = e1->next;
- e1->next = e.release ();
- }
-}
-
-/* Return non-zero if we're stopped at a file to be skipped. */
-
-static int
-skip_file_p (struct skiplist_entry *e,
- const struct symtab_and_line *function_sal)
+bool
+skiplist_entry::do_skip_file_p (const symtab_and_line &function_sal) const
{
- 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.c_str ()))
- return 1;
+ if (compare_filenames_for_search (function_sal.symtab->filename,
+ m_file.c_str ()))
+ return true;

/* 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.c_str ())) != 0)
- return 0;
+ && filename_cmp (lbasename (function_sal.symtab->filename),
+ lbasename (m_file.c_str ())) != 0)
+ return false;

/* Note: symtab_to_fullname caches its result, thus we don't have to. */
{
- const char *fullname = symtab_to_fullname (function_sal->symtab);
+ const char *fullname = symtab_to_fullname (function_sal.symtab);

- if (compare_filenames_for_search (fullname, e->file.c_str ()))
- return 1;
+ if (compare_filenames_for_search (fullname, m_file.c_str ()))
+ return true;
}

- return 0;
+ return false;
}

-/* Return non-zero if we're stopped at a globbed file to be skipped. */
-
-static int
-skip_gfile_p (struct skiplist_entry *e,
- const struct symtab_and_line *function_sal)
+bool
+skiplist_entry::do_skip_gfile_p (const symtab_and_line &function_sal) const
{
- 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.c_str (), function_sal->symtab->filename,
+ if (gdb_filename_fnmatch (m_file.c_str (), function_sal.symtab->filename,
FNM_FILE_NAME | FNM_NOESCAPE) == 0)
- return 1;
+ return true;

/* Before we invoke symtab_to_fullname, which is expensive, do a quick
comparison of the basenames.
@@ -533,101 +521,83 @@ 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.c_str ()),
- lbasename (function_sal->symtab->filename),
+ && gdb_filename_fnmatch (lbasename (m_file.c_str ()),
+ lbasename (function_sal.symtab->filename),
FNM_FILE_NAME | FNM_NOESCAPE) != 0)
- return 0;
+ return false;

/* Note: symtab_to_fullname caches its result, thus we don't have to. */
{
- const char *fullname = symtab_to_fullname (function_sal->symtab);
+ const char *fullname = symtab_to_fullname (function_sal.symtab);

- if (compare_glob_filenames_for_search (fullname, e->file.c_str ()))
- return 1;
+ if (compare_glob_filenames_for_search (fullname, m_file.c_str ()))
+ return true;
}

- return 0;
+ return false;
}

-/* Return non-zero if we're stopped at a function to be skipped. */
-
-static int
-skip_function_p (struct skiplist_entry *e, const char *function_name)
+bool
+skiplist_entry::skip_file_p (const symtab_and_line &function_sal) const
{
- gdb_assert (!e->function.empty () && !e->function_is_regexp);
- return strcmp_iw (function_name, e->function.c_str ()) == 0;
-}
+ if (m_file.empty ())
+ return false;

-/* Return non-zero if we're stopped at a function regexp to be skipped. */
+ if (function_sal.symtab == NULL)
+ return false;
+
+ if (m_file_is_glob)
+ return do_skip_gfile_p (function_sal);
+ else
+ return do_skip_file_p (function_sal);
+}

-static int
-skip_rfunction_p (struct skiplist_entry *e, const char *function_name)
+bool
+skiplist_entry::skip_function_p (const char *function_name) const
{
- 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);
+ if (m_function.empty ())
+ return false;
+
+ if (m_function_is_regexp)
+ {
+ gdb_assert (m_compiled_function_regexp);
+ return (m_compiled_function_regexp->exec (function_name, 0, NULL, 0)
+ == 0);
+ }
+ else
+ return strcmp_iw (function_name, m_function.c_str ()) == 0;
}

/* See skip.h. */

-int
+bool
function_name_is_marked_for_skip (const char *function_name,
- const struct symtab_and_line *function_sal)
+ const symtab_and_line &function_sal)
{
- struct skiplist_entry *e;
-
if (function_name == NULL)
- return 0;
+ return false;

- ALL_SKIPLIST_ENTRIES (e)
+ for (const skiplist_entry &e : skiplist_entries)
{
- int skip_by_file = 0;
- int skip_by_function = 0;
-
- if (!e->enabled)
+ if (!e.enabled ())
continue;

- if (!e->file.empty ())
- {
- if (e->file_is_glob)
- {
- if (skip_gfile_p (e, function_sal))
- skip_by_file = 1;
- }
- else
- {
- if (skip_file_p (e, function_sal))
- skip_by_file = 1;
- }
- }
- if (!e->function.empty ())
- {
- if (e->function_is_regexp)
- {
- if (skip_rfunction_p (e, function_name))
- skip_by_function = 1;
- }
- else
- {
- if (skip_function_p (e, function_name))
- skip_by_function = 1;
- }
- }
+ bool skip_by_file = e.skip_file_p (function_sal);
+ bool skip_by_function = e.skip_function_p (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.empty () && !e->function.empty ())
+ if (!e.file ().empty () && !e.function ().empty ())
{
if (skip_by_file && skip_by_function)
- return 1;
+ return true;
}
/* Only one of file/function is specified. */
else if (skip_by_file || skip_by_function)
- return 1;
+ return true;
}

- return 0;
+ return false;
}

/* Provide a prototype to silence -Wmissing-prototypes. */
@@ -639,9 +609,6 @@ _initialize_step_skip (void)
static struct cmd_list_element *skiplist = NULL;
struct cmd_list_element *c;

- skiplist_entry_chain = 0;
- skiplist_entry_count = 0;
-
add_prefix_cmd ("skip", class_breakpoint, skip_command, _("\
Ignore a function while stepping.\n\
\n\
diff --git a/gdb/skip.h b/gdb/skip.h
index dbc92d8..4e1b544 100644
--- a/gdb/skip.h
+++ b/gdb/skip.h
@@ -20,9 +20,9 @@

struct symtab_and_line;

-/* Returns 1 if the given FUNCTION_NAME is marked for skip and shouldn't be
- stepped into. Otherwise, returns 0. */
-int function_name_is_marked_for_skip (const char *function_name,
- const struct symtab_and_line *function_sal);
+/* Returns true if the given FUNCTION_NAME is marked for skip and
+ shouldn't be stepped into. Otherwise, returns false. */
+bool function_name_is_marked_for_skip (const char *function_name,
+ const symtab_and_line &function_sal);

#endif /* !defined (SKIP_H) */
--
2.5.5