From patchwork Mon Oct 9 23:47:56 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tom Tromey X-Patchwork-Id: 23421 Received: (qmail 903 invoked by alias); 9 Oct 2017 23:48:37 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 110329 invoked by uid 89); 9 Oct 2017 23:48:22 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-25.0 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.2 spammy=Saved, silent, CMD, sk:downloa X-HELO: gproxy6-pub.mail.unifiedlayer.com Received: from gproxy6-pub.mail.unifiedlayer.com (HELO gproxy6-pub.mail.unifiedlayer.com) (67.222.39.168) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 09 Oct 2017 23:48:09 +0000 Received: from cmgw2 (unknown [10.0.90.83]) by gproxy6.mail.unifiedlayer.com (Postfix) with ESMTP id DCF571E091E for ; Mon, 9 Oct 2017 17:48:06 -0600 (MDT) Received: from box522.bluehost.com ([74.220.219.122]) by cmgw2 with id Kbo31w00b2f2jeq01bo6zb; Mon, 09 Oct 2017 17:48:06 -0600 X-Authority-Analysis: v=2.2 cv=dZfw5Tfe c=1 sm=1 tr=0 a=GsOEXm/OWkKvwdLVJsfwcA==:117 a=GsOEXm/OWkKvwdLVJsfwcA==:17 a=02M-m0pO-4AA:10 a=zstS-IiYAAAA:8 a=ZREupXmQg2CPLQPtldsA:9 a=RTNr2f90HBpWneOJ:21 a=qSIMA_qvfn3pst2h:21 a=4G6NA9xxw8l3yy4pmD5M:22 Received: from 184-96-33-178.hlrn.qwest.net ([184.96.33.178]:44720 helo=bapiya.localdomain) by box522.bluehost.com with esmtpsa (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.87) (envelope-from ) id 1e1hmF-0048rI-Jt; Mon, 09 Oct 2017 17:48:03 -0600 From: Tom Tromey To: gdb-patches@sourceware.org Cc: Tom Tromey Subject: [RFA 1/2] Remove some cleanups from breakpoint.c Date: Mon, 9 Oct 2017 17:47:56 -0600 Message-Id: <20171009234757.14624-2-tom@tromey.com> In-Reply-To: <20171009234757.14624-1-tom@tromey.com> References: <20171009234757.14624-1-tom@tromey.com> X-BWhitelist: no X-Exim-ID: 1e1hmF-0048rI-Jt X-Source-Sender: 184-96-33-178.hlrn.qwest.net (bapiya.localdomain) [184.96.33.178]:44720 X-Source-Auth: tom+tromey.com X-Email-Count: 3 X-Source-Cap: ZWx5bnJvYmk7ZWx5bnJvYmk7Ym94NTIyLmJsdWVob3N0LmNvbQ== X-Local-Domain: yes This removes some cleanups from breakpoint.c, replacing them with C++ data structures. ChangeLog 2017-10-09 Tom Tromey * breakpoint.c (commands_command_1): Use std::string. (cleanup_executing_breakpoints): Remove. (bpstat_do_actions_1): Use scoped_restore. (bpstat_check_watchpoint): Use std::string. (decode_static_tracepoint_spec): Likewise. (break_range_command): Likewise. (watch_command_1): Likewise. (compare_breakpoints): Change argument types. (clear_command): Use std::vector. (cleanup_executing_breakpoints): Remove. (update_global_location_list): Use unique_xmalloc_ptr. (strace_command): Remove unused declaration. --- gdb/ChangeLog | 15 ++++++ gdb/breakpoint.c | 148 ++++++++++++++++++------------------------------------- 2 files changed, 64 insertions(+), 99 deletions(-) diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index 59cb3547cf..e56858286c 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -180,8 +180,6 @@ static void info_watchpoints_command (char *, int); static int breakpoint_cond_eval (void *); -static void cleanup_executing_breakpoints (void *); - static void commands_command (char *, int); static void condition_command (char *, int); @@ -1294,22 +1292,16 @@ commands_command_1 (const char *arg, int from_tty, cmd = copy_command_lines (control->body_list[0]); else { - struct cleanup *old_chain; - char *str; - - str = xstrprintf (_("Type commands for breakpoint(s) " - "%s, one per line."), - arg); + std::string str + = string_printf (_("Type commands for breakpoint(s) " + "%s, one per line."), + arg); - old_chain = make_cleanup (xfree, str); - - cmd = read_command_lines (str, + cmd = read_command_lines (&str[0], from_tty, 1, (is_tracepoint (b) ? check_tracepoint_command : 0), b); - - do_cleanups (old_chain); } } @@ -4508,14 +4500,6 @@ breakpoint_about_to_proceed (void) breakpoint_proceeded = 1; } -/* Stub for cleaning up our state if we error-out of a breakpoint - command. */ -static void -cleanup_executing_breakpoints (void *ignore) -{ - executing_breakpoint_commands = 0; -} - /* Return non-zero iff CMD as the first line of a command sequence is `silent' or its equivalent. */ @@ -4538,7 +4522,6 @@ static int bpstat_do_actions_1 (bpstat *bsp) { bpstat bs; - struct cleanup *old_chain; int again = 0; /* Avoid endless recursion if a `source' command is contained @@ -4546,8 +4529,8 @@ bpstat_do_actions_1 (bpstat *bsp) if (executing_breakpoint_commands) return 0; - executing_breakpoint_commands = 1; - old_chain = make_cleanup (cleanup_executing_breakpoints, 0); + scoped_restore save_executing + = make_scoped_restore (&executing_breakpoint_commands, 1); scoped_restore preventer = prevent_dont_repeat (); @@ -4614,7 +4597,6 @@ bpstat_do_actions_1 (bpstat *bsp) break; } } - do_cleanups (old_chain); return again; } @@ -5185,13 +5167,11 @@ bpstat_check_watchpoint (bpstat bs) if (must_check_value) { - char *message - = xstrprintf ("Error evaluating expression for watchpoint %d\n", - b->number); - struct cleanup *cleanups = make_cleanup (xfree, message); - int e = catch_errors (watchpoint_check, bs, message, + std::string message + = string_printf ("Error evaluating expression for watchpoint %d\n", + b->number); + int e = catch_errors (watchpoint_check, bs, message.c_str (), RETURN_MASK_ALL); - do_cleanups (cleanups); switch (e) { case WP_DELETED: @@ -9433,22 +9413,20 @@ static std::vector decode_static_tracepoint_spec (const char **arg_p) { VEC(static_tracepoint_marker_p) *markers = NULL; - struct cleanup *old_chain; const char *p = &(*arg_p)[3]; const char *endp; - char *marker_str; int i; p = skip_spaces (p); endp = skip_to_space (p); - marker_str = savestring (p, endp - p); - old_chain = make_cleanup (xfree, marker_str); + std::string marker_str (p, endp - p); - markers = target_static_tracepoint_markers_by_strid (marker_str); + markers = target_static_tracepoint_markers_by_strid (marker_str.c_str ()); if (VEC_empty(static_tracepoint_marker_p, markers)) - error (_("No known static tracepoint marker named %s"), marker_str); + error (_("No known static tracepoint marker named %s"), + marker_str.c_str ()); std::vector sals; sals.reserve (VEC_length(static_tracepoint_marker_p, markers)); @@ -9466,8 +9444,6 @@ decode_static_tracepoint_spec (const char **arg_p) release_static_tracepoint_marker (marker); } - do_cleanups (old_chain); - *arg_p = endp; return sals; } @@ -10068,12 +10044,10 @@ break_range_command (char *arg_in, int from_tty) { const char *arg = arg_in; const char *arg_start; - char *addr_string_start; struct linespec_result canonical_start, canonical_end; int bp_count, can_use_bp, length; CORE_ADDR end; struct breakpoint *b; - struct cleanup *cleanup_bkpt; /* We don't support software ranged breakpoints. */ if (target_ranged_break_num_registers () < 0) @@ -10107,8 +10081,7 @@ break_range_command (char *arg_in, int from_tty) error (_("Cannot create a ranged breakpoint with multiple locations.")); const symtab_and_line &sal_start = lsal_start.sals[0]; - addr_string_start = savestring (arg_start, arg - arg_start); - cleanup_bkpt = make_cleanup (xfree, addr_string_start); + std::string addr_string_start (arg_start, arg - arg_start); arg++; /* Skip the comma. */ arg = skip_spaces (arg); @@ -10150,9 +10123,7 @@ break_range_command (char *arg_in, int from_tty) { /* This range is simple enough to be handled by the `hbreak' command. */ - hbreak_command (addr_string_start, 1); - - do_cleanups (cleanup_bkpt); + hbreak_command (&addr_string_start[0], 1); return; } @@ -10167,8 +10138,6 @@ break_range_command (char *arg_in, int from_tty) b->location_range_end = std::move (end_location); b->loc->length = length; - do_cleanups (cleanup_bkpt); - mention (b); observer_notify_breakpoint_created (b); update_global_location_list (UGLL_MAY_INSERT); @@ -10793,8 +10762,6 @@ watch_command_1 (const char *arg, int accessflag, int from_tty, the hardware watchpoint. */ int use_mask = 0; CORE_ADDR mask = 0; - char *expression; - struct cleanup *back_to; /* Make sure that we actually have parameters to parse. */ if (arg != NULL && arg[0] != '\0') @@ -10883,9 +10850,8 @@ watch_command_1 (const char *arg, int accessflag, int from_tty, is in terms of a newly allocated string instead of the original ARG. */ innermost_block = NULL; - expression = savestring (arg, exp_end - arg); - back_to = make_cleanup (xfree, expression); - exp_start = arg = expression; + std::string expression (arg, exp_end - arg); + exp_start = arg = expression.c_str (); expression_up exp = parse_exp_1 (&arg, 0, 0, 0); exp_end = arg; /* Remove trailing whitespace from the expression before saving it. @@ -11085,7 +11051,6 @@ watch_command_1 (const char *arg, int accessflag, int from_tty, update_watchpoint (w.get (), 1); install_breakpoint (internal, std::move (w), 1); - do_cleanups (back_to); } /* Return count of debug registers needed to watch the given expression. @@ -11607,19 +11572,17 @@ tcatch_command (char *arg, int from_tty) error (_("Catch requires an event name.")); } -/* A qsort comparison function that sorts breakpoints in order. */ +/* Compare two breakpoints and return a strcmp-like result. */ static int -compare_breakpoints (const void *a, const void *b) +compare_breakpoints (const breakpoint_p &a, const breakpoint_p &b) { - const breakpoint_p *ba = (const breakpoint_p *) a; - uintptr_t ua = (uintptr_t) *ba; - const breakpoint_p *bb = (const breakpoint_p *) b; - uintptr_t ub = (uintptr_t) *bb; + uintptr_t ua = (uintptr_t) a; + uintptr_t ub = (uintptr_t) b; - if ((*ba)->number < (*bb)->number) + if (a->number < b->number) return -1; - else if ((*ba)->number > (*bb)->number) + else if (a->number > b->number) return 1; /* Now sort by address, in case we see, e..g, two breakpoints with @@ -11634,12 +11597,9 @@ compare_breakpoints (const void *a, const void *b) static void clear_command (char *arg, int from_tty) { - struct breakpoint *b, *prev; - VEC(breakpoint_p) *found = 0; - int ix; + struct breakpoint *b; int default_match; int i; - struct cleanup *cleanups = make_cleanup (null_cleanup, NULL); std::vector decoded_sals; symtab_and_line last_sal; @@ -11688,8 +11648,7 @@ clear_command (char *arg, int from_tty) from_tty is forced true if we delete more than one breakpoint. */ - found = NULL; - make_cleanup (VEC_cleanup (breakpoint_p), &found); + std::vector found; for (const auto &sal : sals) { const char *sal_fullname; @@ -11747,12 +11706,12 @@ clear_command (char *arg, int from_tty) } if (match) - VEC_safe_push(breakpoint_p, found, b); + found.push_back (b); } } /* Now go thru the 'found' chain and delete them. */ - if (VEC_empty(breakpoint_p, found)) + if (found.empty ()) { if (arg) error (_("No breakpoint at %s."), arg); @@ -11761,40 +11720,36 @@ clear_command (char *arg, int from_tty) } /* Remove duplicates from the vec. */ - qsort (VEC_address (breakpoint_p, found), - VEC_length (breakpoint_p, found), - sizeof (breakpoint_p), - compare_breakpoints); - prev = VEC_index (breakpoint_p, found, 0); - for (ix = 1; VEC_iterate (breakpoint_p, found, ix, b); ++ix) - { - if (b == prev) - { - VEC_ordered_remove (breakpoint_p, found, ix); - --ix; - } - } + std::sort (found.begin (), found.end (), + [=] (const breakpoint_p &a, const breakpoint_p &b) + { + return compare_breakpoints (a, b) < 0; + }); + found.erase (std::unique (found.begin (), found.end (), + [=] (const breakpoint_p &a, const breakpoint_p &b) + { + return compare_breakpoints (a, b) == 0; + }), + found.end ()); - if (VEC_length(breakpoint_p, found) > 1) + if (found.size () > 1) from_tty = 1; /* Always report if deleted more than one. */ if (from_tty) { - if (VEC_length(breakpoint_p, found) == 1) + if (found.size () == 1) printf_unfiltered (_("Deleted breakpoint ")); else printf_unfiltered (_("Deleted breakpoints ")); } - for (ix = 0; VEC_iterate(breakpoint_p, found, ix, b); ix++) + for (breakpoint_p iter : found) { if (from_tty) - printf_unfiltered ("%d ", b->number); - delete_breakpoint (b); + printf_unfiltered ("%d ", iter->number); + delete_breakpoint (iter); } if (from_tty) putchar_unfiltered ('\n'); - - do_cleanups (cleanups); } /* Delete breakpoint in BS if they are `delete' breakpoints and @@ -12035,7 +11990,6 @@ update_global_location_list (enum ugll_insert_mode insert_mode) { struct breakpoint *b; struct bp_location **locp, *loc; - struct cleanup *cleanups; /* Last breakpoint location address that was marked for update. */ CORE_ADDR last_addr = 0; /* Last breakpoint location program space that was marked for update. */ @@ -12054,14 +12008,13 @@ update_global_location_list (enum ugll_insert_mode insert_mode) /* Saved former bp_locations array which we compare against the newly built bp_locations from the current state of ALL_BREAKPOINTS. */ - struct bp_location **old_locations, **old_locp; + struct bp_location **old_locp; unsigned old_locations_count; + gdb::unique_xmalloc_ptr old_locations (bp_locations); - old_locations = bp_locations; old_locations_count = bp_locations_count; bp_locations = NULL; bp_locations_count = 0; - cleanups = make_cleanup (xfree, old_locations); ALL_BREAKPOINTS (b) for (loc = b->loc; loc; loc = loc->next) @@ -12088,8 +12041,8 @@ update_global_location_list (enum ugll_insert_mode insert_mode) and former bp_location array state respectively. */ locp = bp_locations; - for (old_locp = old_locations; - old_locp < old_locations + old_locations_count; + for (old_locp = old_locations.get (); + old_locp < old_locations.get () + old_locations_count; old_locp++) { struct bp_location *old_loc = *old_locp; @@ -12372,8 +12325,6 @@ update_global_location_list (enum ugll_insert_mode insert_mode) if (insert_mode != UGLL_DONT_INSERT) download_tracepoint_locations (); - - do_cleanups (cleanups); } void @@ -14820,7 +14771,6 @@ strace_command (char *arg_in, int from_tty) const char *arg = arg_in; struct breakpoint_ops *ops; event_location_up location; - struct cleanup *back_to; /* Decide if we are dealing with a static tracepoint marker (`-m'), or with a normal static tracepoint. */