* [RFA 2/2] Remove prepare_re_set_context
2017-10-09 23:48 [RFA 0/2] some simple cleanup removal in breakpoint.c Tom Tromey
@ 2017-10-09 23:48 ` Tom Tromey
2017-10-10 18:56 ` Simon Marchi
2017-10-09 23:48 ` [RFA 1/2] Remove some cleanups from breakpoint.c Tom Tromey
1 sibling, 1 reply; 6+ messages in thread
From: Tom Tromey @ 2017-10-09 23:48 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
prepare_re_set_context returns a null cleanup and doesn't seem
generally useful. This patch removes it plus a few more cleanups; and
changes breakpoint_re_set to use scoped_restore rather than its own
manual mechanism.
ChangeLog
2017-10-09 Tom Tromey <tom@tromey.com>
* breakpoint.c (prepare_re_set_context): Remove.
(breakpoint_re_set_one): Update. Don't use cleanups.
(breakpoint_re_set): Use scoped_restore, std::string, and
scoped_restore_current_language.
---
gdb/ChangeLog | 7 +++++++
gdb/breakpoint.c | 36 ++++++++++--------------------------
2 files changed, 17 insertions(+), 26 deletions(-)
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index e56858286c..2eba1e9c48 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -14088,17 +14088,6 @@ decode_location_default (struct breakpoint *b,
return {};
}
-/* Prepare the global context for a re-set of breakpoint B. */
-
-static struct cleanup *
-prepare_re_set_context (struct breakpoint *b)
-{
- input_radix = b->input_radix;
- set_language (b->language);
-
- return make_cleanup (null_cleanup, NULL);
-}
-
/* Reset a breakpoint given it's struct breakpoint * BINT.
The value we return ends up being the return value from catch_errors.
Unused in this case. */
@@ -14108,11 +14097,11 @@ breakpoint_re_set_one (void *bint)
{
/* Get past catch_errs. */
struct breakpoint *b = (struct breakpoint *) bint;
- struct cleanup *cleanups;
- cleanups = prepare_re_set_context (b);
+ input_radix = b->input_radix;
+ set_language (b->language);
+
b->ops->re_set (b);
- do_cleanups (cleanups);
return 0;
}
@@ -14123,13 +14112,10 @@ void
breakpoint_re_set (void)
{
struct breakpoint *b, *b_tmp;
- enum language save_language;
- int save_input_radix;
-
- save_language = current_language->la_language;
- save_input_radix = input_radix;
{
+ scoped_restore_current_language save_language;
+ scoped_restore save_input_radix = make_scoped_restore (&input_radix);
scoped_restore_current_pspace_and_thread restore_pspace_thread;
/* Note: we must not try to insert locations until after all
@@ -14140,14 +14126,12 @@ breakpoint_re_set (void)
ALL_BREAKPOINTS_SAFE (b, b_tmp)
{
/* Format possible error msg. */
- char *message = xstrprintf ("Error in re-setting breakpoint %d: ",
- b->number);
- struct cleanup *cleanups = make_cleanup (xfree, message);
- catch_errors (breakpoint_re_set_one, b, message, RETURN_MASK_ALL);
- do_cleanups (cleanups);
+ std::string message
+ = string_printf ("Error in re-setting breakpoint %d: ",
+ b->number);
+ catch_errors (breakpoint_re_set_one, b, message.c_str (),
+ RETURN_MASK_ALL);
}
- set_language (save_language);
- input_radix = save_input_radix;
jit_breakpoint_re_set ();
}
--
2.13.6
^ permalink raw reply [flat|nested] 6+ messages in thread
* [RFA 1/2] Remove some cleanups from breakpoint.c
2017-10-09 23:48 [RFA 0/2] some simple cleanup removal in breakpoint.c Tom Tromey
2017-10-09 23:48 ` [RFA 2/2] Remove prepare_re_set_context Tom Tromey
@ 2017-10-09 23:48 ` Tom Tromey
2017-10-10 18:25 ` Simon Marchi
1 sibling, 1 reply; 6+ messages in thread
From: Tom Tromey @ 2017-10-09 23:48 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
This removes some cleanups from breakpoint.c, replacing them with C++
data structures.
ChangeLog
2017-10-09 Tom Tromey <tom@tromey.com>
* 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<symtab_and_line>
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<symtab_and_line> 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<symtab_and_line> 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<struct breakpoint *> 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);
}
\f
/* 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<struct bp_location *> 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. */
--
2.13.6
^ permalink raw reply [flat|nested] 6+ messages in thread
* [RFA 0/2] some simple cleanup removal in breakpoint.c
@ 2017-10-09 23:48 Tom Tromey
2017-10-09 23:48 ` [RFA 2/2] Remove prepare_re_set_context Tom Tromey
2017-10-09 23:48 ` [RFA 1/2] Remove some cleanups from breakpoint.c Tom Tromey
0 siblings, 2 replies; 6+ messages in thread
From: Tom Tromey @ 2017-10-09 23:48 UTC (permalink / raw)
To: gdb-patches
This short series removes a number of simpler cleanups from
breakpoint.c. I split it in two because it seemed easier to review
the second patch in isolation.
Regression tested by the buildbot.
Tom
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFA 1/2] Remove some cleanups from breakpoint.c
2017-10-09 23:48 ` [RFA 1/2] Remove some cleanups from breakpoint.c Tom Tromey
@ 2017-10-10 18:25 ` Simon Marchi
2017-10-11 21:44 ` Tom Tromey
0 siblings, 1 reply; 6+ messages in thread
From: Simon Marchi @ 2017-10-10 18:25 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
On 2017-10-09 19:47, Tom Tromey wrote:
> @@ -9433,22 +9413,20 @@ static std::vector<symtab_and_line>
> 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 ());
It's not related to your patch, but is it possible that the markers VEC
never gets freed?
> @@ -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)
You don't actually need to capture anything in these two lambdas.
IIUC, using "breakpoint_p" was only needed because of the VEC. Now that
we use std::vector, it would be nice if you changed breakpoint_p to
breakpoint * in the code you touched.
With those changed, the patch LGTM.
Thanks,
Simon
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFA 2/2] Remove prepare_re_set_context
2017-10-09 23:48 ` [RFA 2/2] Remove prepare_re_set_context Tom Tromey
@ 2017-10-10 18:56 ` Simon Marchi
0 siblings, 0 replies; 6+ messages in thread
From: Simon Marchi @ 2017-10-10 18:56 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
On 2017-10-09 19:47, Tom Tromey wrote:
> prepare_re_set_context returns a null cleanup and doesn't seem
> generally useful. This patch removes it plus a few more cleanups; and
> changes breakpoint_re_set to use scoped_restore rather than its own
> manual mechanism.
>
> ChangeLog
> 2017-10-09 Tom Tromey <tom@tromey.com>
>
> * breakpoint.c (prepare_re_set_context): Remove.
> (breakpoint_re_set_one): Update. Don't use cleanups.
> (breakpoint_re_set): Use scoped_restore, std::string, and
> scoped_restore_current_language.
> ---
> gdb/ChangeLog | 7 +++++++
> gdb/breakpoint.c | 36 ++++++++++--------------------------
> 2 files changed, 17 insertions(+), 26 deletions(-)
>
> diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
> index e56858286c..2eba1e9c48 100644
> --- a/gdb/breakpoint.c
> +++ b/gdb/breakpoint.c
> @@ -14088,17 +14088,6 @@ decode_location_default (struct breakpoint *b,
> return {};
> }
>
> -/* Prepare the global context for a re-set of breakpoint B. */
> -
> -static struct cleanup *
> -prepare_re_set_context (struct breakpoint *b)
> -{
> - input_radix = b->input_radix;
> - set_language (b->language);
> -
> - return make_cleanup (null_cleanup, NULL);
> -}
> -
> /* Reset a breakpoint given it's struct breakpoint * BINT.
> The value we return ends up being the return value from
> catch_errors.
> Unused in this case. */
> @@ -14108,11 +14097,11 @@ breakpoint_re_set_one (void *bint)
> {
> /* Get past catch_errs. */
> struct breakpoint *b = (struct breakpoint *) bint;
> - struct cleanup *cleanups;
>
> - cleanups = prepare_re_set_context (b);
> + input_radix = b->input_radix;
> + set_language (b->language);
> +
> b->ops->re_set (b);
> - do_cleanups (cleanups);
> return 0;
> }
>
> @@ -14123,13 +14112,10 @@ void
> breakpoint_re_set (void)
> {
> struct breakpoint *b, *b_tmp;
> - enum language save_language;
> - int save_input_radix;
> -
> - save_language = current_language->la_language;
> - save_input_radix = input_radix;
>
> {
> + scoped_restore_current_language save_language;
> + scoped_restore save_input_radix = make_scoped_restore
> (&input_radix);
> scoped_restore_current_pspace_and_thread restore_pspace_thread;
>
> /* Note: we must not try to insert locations until after all
> @@ -14140,14 +14126,12 @@ breakpoint_re_set (void)
> ALL_BREAKPOINTS_SAFE (b, b_tmp)
> {
> /* Format possible error msg. */
> - char *message = xstrprintf ("Error in re-setting breakpoint %d: ",
> - b->number);
> - struct cleanup *cleanups = make_cleanup (xfree, message);
> - catch_errors (breakpoint_re_set_one, b, message, RETURN_MASK_ALL);
> - do_cleanups (cleanups);
> + std::string message
> + = string_printf ("Error in re-setting breakpoint %d: ",
> + b->number);
> + catch_errors (breakpoint_re_set_one, b, message.c_str (),
> + RETURN_MASK_ALL);
> }
> - set_language (save_language);
> - input_radix = save_input_radix;
>
> jit_breakpoint_re_set ();
> }
LGTM.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFA 1/2] Remove some cleanups from breakpoint.c
2017-10-10 18:25 ` Simon Marchi
@ 2017-10-11 21:44 ` Tom Tromey
0 siblings, 0 replies; 6+ messages in thread
From: Tom Tromey @ 2017-10-11 21:44 UTC (permalink / raw)
To: Simon Marchi; +Cc: Tom Tromey, gdb-patches
>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:
>> + markers = target_static_tracepoint_markers_by_strid
>> (marker_str.c_str ());
Simon> It's not related to your patch, but is it possible that the markers
Simon> VEC never gets freed?
Yes. In fact I think all callers of
target_static_tracepoint_markers_by_strid leak memory.
Simon> You don't actually need to capture anything in these two lambdas.
Simon> IIUC, using "breakpoint_p" was only needed because of the VEC. Now
Simon> that we use std::vector, it would be nice if you changed breakpoint_p
Simon> to breakpoint * in the code you touched.
Simon> With those changed, the patch LGTM.
Thanks, I'll make those changes & push.
Tom
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-10-11 21:44 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-09 23:48 [RFA 0/2] some simple cleanup removal in breakpoint.c Tom Tromey
2017-10-09 23:48 ` [RFA 2/2] Remove prepare_re_set_context Tom Tromey
2017-10-10 18:56 ` Simon Marchi
2017-10-09 23:48 ` [RFA 1/2] Remove some cleanups from breakpoint.c Tom Tromey
2017-10-10 18:25 ` Simon Marchi
2017-10-11 21:44 ` Tom Tromey
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).