Re: [RFA 06/13] Replace start_rbreak_breakpoints and end_rbreak_breakpoints

Tom Tromey
2017-11-03T16:58:15Z

>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:

Simon> Your patch LGTM. But how is the "string" string freed in
Simon> rbreak_command?

It is leaked, which apparently is a regression introduced by an earlier
patch of mine. Sorry about that! I've updated the patch as appended to
fix this problem.

Tom

commit 391e4d7aa45fc4947632aa06a7aa462eeef3377c
Author: Tom Tromey <tom@tromey.com>
Date: Wed Nov 1 09:00:09 2017 -0600

Replace start_rbreak_breakpoints and end_rbreak_breakpoints

This replaces start_rbreak_breakpoints and end_rbreak_breakpoints with
a new scoped class. This allows the removal of a cleanup.

This also fixes an earlier memory leak regression, by changing
"string" to be a std::string.

gdb/ChangeLog
2017-11-03 Tom Tromey <tom@tromey.com>

* breakpoint.c
(scoped_rbreak_breakpoints::scoped_rbreak_breakpoints): Rename
from start_rbreak_breakpoints.
(scoped_rbreak_breakpoints): Rename from end_rbreak_breakpoints.
* breakpoint.h (class scoped_rbreak_breakpoints): New.
(start_rbreak_breakpoints, end_rbreak_breakpoints): Remove.
* symtab.c (do_end_rbreak_breakpoints): Remove.
(rbreak_command): Use scoped_rbreak_breakpoints, std::string.

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index ce6c8db631..56b7e903bc 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,14 @@
+2017-11-03 Tom Tromey <tom@tromey.com>
+
+ * breakpoint.c
+ (scoped_rbreak_breakpoints::scoped_rbreak_breakpoints): Rename
+ from start_rbreak_breakpoints.
+ (scoped_rbreak_breakpoints): Rename from end_rbreak_breakpoints.
+ * breakpoint.h (class scoped_rbreak_breakpoints): New.
+ (start_rbreak_breakpoints, end_rbreak_breakpoints): Remove.
+ * symtab.c (do_end_rbreak_breakpoints): Remove.
+ (rbreak_command): Use scoped_rbreak_breakpoints, std::string.
+
2017-11-02 Tom Tromey <tom@tromey.com>

* cp-namespace.c (reset_directive_searched): Remove.
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 0bf47d5f56..61e41283b1 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -644,8 +644,7 @@ static int rbreak_start_breakpoint_count;
/* Called at the start an "rbreak" command to record the first
breakpoint made. */

-void
-start_rbreak_breakpoints (void)
+scoped_rbreak_breakpoints::scoped_rbreak_breakpoints ()
{
rbreak_start_breakpoint_count = breakpoint_count;
}
@@ -653,8 +652,7 @@ start_rbreak_breakpoints (void)
/* Called at the end of an "rbreak" command to record the last
breakpoint made. */

-void
-end_rbreak_breakpoints (void)
+scoped_rbreak_breakpoints::~scoped_rbreak_breakpoints ()
{
prev_breakpoint_count = rbreak_start_breakpoint_count;
}
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index 9a9433bd66..69b63ea932 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -1601,10 +1601,18 @@ extern VEC(breakpoint_p) *static_tracepoints_here (CORE_ADDR addr);
that each command is suitable for tracepoint command list. */
extern void check_tracepoint_command (char *line, void *closure);

-/* Call at the start and end of an "rbreak" command to register
- breakpoint numbers for a later "commands" command. */
-extern void start_rbreak_breakpoints (void);
-extern void end_rbreak_breakpoints (void);
+/* Create an instance of this to start registering breakpoint numbers
+ for a later "commands" command. */
+
+class scoped_rbreak_breakpoints
+{
+public:
+
+ scoped_rbreak_breakpoints ();
+ ~scoped_rbreak_breakpoints ();
+
+ DISABLE_COPY_AND_ASSIGN (scoped_rbreak_breakpoints);
+};

/* Breakpoint iterator function.

diff --git a/gdb/symtab.c b/gdb/symtab.c
index 16a6b2eb6f..25b3de0dfb 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -4506,20 +4506,10 @@ rbreak_command_wrapper (char *regexp, int from_tty)
rbreak_command (regexp, from_tty);
}

-/* A cleanup function that calls end_rbreak_breakpoints. */
-
-static void
-do_end_rbreak_breakpoints (void *ignore)
-{
- end_rbreak_breakpoints ();
-}
-
static void
rbreak_command (char *regexp, int from_tty)
{
- struct cleanup *old_chain;
- char *string = NULL;
- int len = 0;
+ std::string string;
const char **files = NULL;
const char *file_name;
int nfiles = 0;
@@ -4550,8 +4540,7 @@ rbreak_command (char *regexp, int from_tty)
FUNCTIONS_DOMAIN,
nfiles, files);

- start_rbreak_breakpoints ();
- old_chain = make_cleanup (do_end_rbreak_breakpoints, NULL);
+ scoped_rbreak_breakpoints finalize;
for (const symbol_search &p : symbols)
{
if (p.msymbol.minsym == NULL)
@@ -4559,20 +4548,9 @@ rbreak_command (char *regexp, int from_tty)
struct symtab *symtab = symbol_symtab (p.symbol);
const char *fullname = symtab_to_fullname (symtab);

- int newlen = (strlen (fullname)
- + strlen (SYMBOL_LINKAGE_NAME (p.symbol))
- + 4);
-
- if (newlen > len)
- {
- string = (char *) xrealloc (string, newlen);
- len = newlen;
- }
- strcpy (string, fullname);
- strcat (string, ":'");
- strcat (string, SYMBOL_LINKAGE_NAME (p.symbol));
- strcat (string, "'");
- break_command (string, from_tty);
+ string = (std::string (fullname) + ":'"
+ + SYMBOL_LINKAGE_NAME (p.symbol) + "'");
+ break_command (&string[0], from_tty);
print_symbol_info (FUNCTIONS_DOMAIN,
p.symbol,
p.block,
@@ -4580,24 +4558,14 @@ rbreak_command (char *regexp, int from_tty)
}
else
{
- int newlen = (strlen (MSYMBOL_LINKAGE_NAME (p.msymbol.minsym)) + 3);
-
- if (newlen > len)
- {
- string = (char *) xrealloc (string, newlen);
- len = newlen;
- }
- strcpy (string, "'");
- strcat (string, MSYMBOL_LINKAGE_NAME (p.msymbol.minsym));
- strcat (string, "'");
+ string = (std::string ("'")
+ + MSYMBOL_LINKAGE_NAME (p.msymbol.minsym) + "'");

- break_command (string, from_tty);
+ break_command (&string[0], from_tty);
printf_filtered ("<function, no debug info> %s;\n",
MSYMBOL_PRINT_NAME (p.msymbol.minsym));
}
}
-
- do_cleanups (old_chain);
}
\f