From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 107112 invoked by alias); 10 Aug 2017 19:51:11 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 107081 invoked by uid 89); 10 Aug 2017 19:51:10 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 10 Aug 2017 19:51:06 +0000 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 9E69FC0546EF; Thu, 10 Aug 2017 19:51:04 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 9E69FC0546EF Authentication-Results: ext-mx08.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx08.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=palves@redhat.com Received: from [127.0.0.1] (ovpn04.gateway.prod.ext.ams2.redhat.com [10.39.146.4]) by smtp.corp.redhat.com (Postfix) with ESMTP id 205AC97BA7; Thu, 10 Aug 2017 19:51:02 +0000 (UTC) Subject: Re: [PATCH] More gdb/skip.c C++ification To: Tom Tromey , Simon Marchi References: <20170806194231.26627-1-tom@tromey.com> <5e21aff786d07501d546194f8f48e5bb@polymtl.ca> <87d187cwf3.fsf@pokyo> <19ea4d685104d39dfc20486425f41f37@polymtl.ca> <87zib9hmfa.fsf@tromey.com> <04c97e6e95cf8f012979b6b1052871a0@polymtl.ca> <87valwh9tf.fsf@tromey.com> <57ff34fe-bafd-8b54-b7a2-eb675241da63@redhat.com> Cc: gdb-patches@sourceware.org From: Pedro Alves Message-ID: <15d542c3-c4be-50dc-bdef-ac5a63cfedee@redhat.com> Date: Thu, 10 Aug 2017 19:51:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <57ff34fe-bafd-8b54-b7a2-eb675241da63@redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-SW-Source: 2017-08/txt/msg00220.txt.bz2 On 08/10/2017 08:34 PM, Pedro Alves wrote: > On 08/09/2017 07:31 PM, Tom Tromey wrote: >>>>>>> "Simon" == Simon Marchi 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 > 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 > > * 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_function_regexp; > + gdb::optional 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 &&e); > +typedef std::unique_ptr skiplist_entry_up; > > -static struct skiplist_entry *skiplist_entry_chain; > -static int skiplist_entry_count; > +static std::list 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 > -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 > - (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 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 () ? "" > - : e->file.c_str ()); /* 4 */ > - if (e->function_is_regexp) > + e->file ().empty () ? "" > + : 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 () ? "" > - : e->function.c_str ()); /* 6 */ > + e->function ().empty () ? "" > + : 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 &&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) */ >