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

Pedro Alves
2017-08-10T19:51:11Z

On 08/10/2017 08:34 PM, Pedro Alves wrote:
> On 08/09/2017 07:31 PM, Tom Tromey wrote:
>>>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:
>>
>> Simon> That would avoid complicating the calling functions as well.
>> Simon> Either way (adding std::moves or this) are fine for me, it's as
>> Simon> you wish.
>>
>> I've added the moves, and I'm checking it in. Thanks.
>>
>
> Thanks much for the fix, and sorry for not realizing I was
> introducing the leak.
>
> I thought I had a local branch somewhere that C++ified skip.c
> some more, and I spent a bit today looking for it, but couldn't
> find it. As penance for introducing the bug in the first place,
> and since I was staring at skip.c, I ended up (re?)coding what I
> thought I had, on top of Tom's patch.
>
> It's a bit of churn, but there's nothing really complicated here.
> It's just the next logical steps after Tom's patch. See commit
> log below.
>
> WDYT?
>
> Tested on GNU/Linux.

Actually, now that I turn my automatic-brainless-C++ification-mode
switch off I can't really explain why I thought of making the entry
chain a std::list of pointers instead of a list of objects... (which
would spare the double allocation [node + element].) I'll see about
changing that...

Thanks,
Pedro Alves

>
> From 94ca8b042a5696e020d614be64f4bb83c1d111e6 Mon Sep 17 00:00:00 2001
> From: Pedro Alves <palves@redhat.com>
> Date: Thu, 10 Aug 2017 19:58:24 +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.
> - Add a skiplist_entry_up typedef and use it more.
> - 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:
> yyyy-mm-dd 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_up): New typedef.
> (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, skiplist_entry_up and getters.
> (skip_enable_command, skip_disable_command): Adjust to use
> range-for, skiplist_entry_up and setters.
> (skip_delete_command): Adjust to use std::list of
> skiplist_entry_up.
> (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,
> skiplist_entry_up 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/infrun.c | 2 +-
> gdb/skip.c | 426 +++++++++++++++++++++++++++--------------------------------
> gdb/skip.h | 8 +-
> 3 files changed, 198 insertions(+), 238 deletions(-)
>
> 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..b10f8fb 100644
> --- a/gdb/skip.c
> +++ b/gdb/skip.c
> @@ -35,74 +35,122 @@
> #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);
> +
> + /* Return true if the skip entry has a function or function regexp
> + that matches FUNCTION_NAME. */
> + bool skip_function_p (const char *function_name);
> +
> + /* 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; };
> +
> +private:
> + /* Use add_entry factory to construct instead. */
> + skiplist_entry (bool file_is_glob, std::string &&file,
> + bool function_is_regexp, std::string &&function);
> +
> + /* Return true if we're stopped at a file to be skipped. */
> + bool do_skip_file_p (const symtab_and_line &function_sal);
> +
> + /* Return true if we're stopped at a globbed file to be skipped. */
> + bool do_skip_gfile_p (const symtab_and_line &function_sal);
> +
> +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);
> +typedef std::unique_ptr<skiplist_entry> skiplist_entry_up;
>
> -static struct skiplist_entry *skiplist_entry_chain;
> -static int skiplist_entry_count;
> +static std::list<skiplist_entry_up> skiplist_entries;
> +static int highest_skiplist_entry_num = 0;
>
> -#define ALL_SKIPLIST_ENTRIES(E) \
> - for (E = skiplist_entry_chain; E; E = E->next)
> +skiplist_entry::skiplist_entry (bool file_is_glob,
> + std::string &&file,
> + bool function_is_regexp,
> + std::string &&function)
> + : 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 ());
>
> -#define ALL_SKIPLIST_ENTRIES_SAFE(E,TMP) \
> - for (E = skiplist_entry_chain; \
> - E ? (TMP = E->next, 1) : 0; \
> - E = TMP)
> + if (m_file_is_glob)
> + gdb_assert (!m_file.empty ());
> +
> + if (m_function_is_regexp)
> + {
> + gdb_assert (!m_function.empty ());
> +
> + 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_entry *e = new skiplist_entry (file_is_glob,
> + std::move (file),
> + function_is_regexp,
> + std::move (function));
> +
> + /* Add to the end of the chain so that the list of skiplist entries
> + is always in numerical order. */
> + skiplist_entries.push_back (skiplist_entry_up (e));
> +
> + /* Incremented after push_back, in case push_back throws. */
> + e->m_number = ++highest_skiplist_entry_num;
> }
>
> static void
> @@ -126,8 +174,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 +186,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 +216,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 +303,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 +348,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 +355,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_up &e : skiplist_entries)
> + if (arg == NULL || number_is_in_list (arg, e->number ()))
> num_printable_entries++;
>
> if (num_printable_entries == 0)
> @@ -355,37 +381,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_up &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 +419,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 (const skiplist_entry_up &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 +435,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 (const skiplist_entry_up &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 +451,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_up &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)
> +bool
> +skiplist_entry::do_skip_file_p (const symtab_and_line &function_sal)
> {
> - 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)
> -{
> - 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)
> {
> - 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 +514,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)
> {
> - 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;
>
> -static int
> -skip_rfunction_p (struct skiplist_entry *e, const char *function_name)
> + if (m_file_is_glob)
> + return do_skip_gfile_p (function_sal);
> + else
> + return do_skip_file_p (function_sal);
> +}
> +
> +bool
> +skiplist_entry::skip_function_p (const char *function_name)
> {
> - 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_up &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 +602,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) */
>