This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFA 3/3] Remove cleanups from break-catch-syscall.c
- From: Simon Marchi <simon dot marchi at ericsson dot com>
- To: Tom Tromey <tom at tromey dot com>, <gdb-patches at sourceware dot org>
- Date: Fri, 20 Oct 2017 16:26:15 -0400
- Subject: Re: [RFA 3/3] Remove cleanups from break-catch-syscall.c
- Authentication-results: sourceware.org; auth=none
- Authentication-results: spf=none (sender IP is ) smtp.mailfrom=simon dot marchi at ericsson dot com;
- References: <20171018040645.7212-1-tom@tromey.com> <20171018040645.7212-3-tom@tromey.com>
- Spamdiagnosticmetadata: NSPM
- Spamdiagnosticoutput: 1:99
On 2017-10-18 12:06 AM, Tom Tromey wrote:
> This removes the remaining cleanups from break-catch-syscall.c by
> storing temporary strings in a vector.
>
> ChangeLog
> 2017-10-17 Tom Tromey <tom@tromey.com>
>
> * break-catch-syscall.c (catch_syscall_completer): Use
> std::string, gdb::unique_xmalloc_ptr.
> ---
> gdb/ChangeLog | 5 +++++
> gdb/break-catch-syscall.c | 35 ++++++++++++++++++-----------------
> 2 files changed, 23 insertions(+), 17 deletions(-)
>
> diff --git a/gdb/break-catch-syscall.c b/gdb/break-catch-syscall.c
> index 01e761ce37..2bbfee0ac4 100644
> --- a/gdb/break-catch-syscall.c
> +++ b/gdb/break-catch-syscall.c
> @@ -560,9 +560,7 @@ catch_syscall_completer (struct cmd_list_element *cmd,
> const char *text, const char *word)
> {
> struct gdbarch *gdbarch = get_current_arch ();
> - struct cleanup *cleanups = make_cleanup (null_cleanup, NULL);
> - const char **group_list = NULL;
> - const char **syscall_list = NULL;
> + gdb::unique_xmalloc_ptr<const char *> group_list;
> const char *prefix;
> int i;
>
> @@ -575,34 +573,37 @@ catch_syscall_completer (struct cmd_list_element *cmd,
> if (startswith (prefix, "g:") || startswith (prefix, "group:"))
> {
> /* Perform completion inside 'group:' namespace only. */
> - group_list = get_syscall_group_names (gdbarch);
> + group_list.reset (get_syscall_group_names (gdbarch));
> if (group_list != NULL)
> - complete_on_enum (tracker, group_list, word, word);
> + complete_on_enum (tracker, group_list.get (), word, word);
> }
> else
> {
> /* Complete with both, syscall names and groups. */
> - syscall_list = get_syscall_names (gdbarch);
> - group_list = get_syscall_group_names (gdbarch);
> + gdb::unique_xmalloc_ptr<const char *> syscall_list
> + (get_syscall_names (gdbarch));
> + group_list.reset (get_syscall_group_names (gdbarch));
> +
> + const char **group_ptr = group_list.get ();
> +
> + /* Hold on to strings while we're using them. */
> + std::vector<std::string> holders;
>
> /* Append "group:" prefix to syscall groups. */
> - for (i = 0; group_list[i] != NULL; i++)
> + for (i = 0; group_ptr[i] != NULL; i++)
> {
> - char *prefixed_group = xstrprintf ("group:%s", group_list[i]);
> + std::string prefixed_group = string_printf ("group:%s",
> + group_ptr[i]);
>
> - group_list[i] = prefixed_group;
> - make_cleanup (xfree, prefixed_group);
> + group_ptr[i] = prefixed_group.c_str ();
> + holders.push_back (prefixed_group);
> }
Err, I think there's something that doesn't make sense here actually. We
record in group_ptr[i] a pointer to the buffer of the temporary std::string,
that gets deleted when we go out of scope (end of the iteration). That
causes this fail:
Running /home/emaisin/src/binutils-gdb/gdb/testsuite/gdb.base/catch-syscall.exp ...
FAIL: gdb.base/catch-syscall.exp: complete catch syscall group suggests 'group:' prefix (pattern 2)
By hand, you can do
(gdb) catch syscall g<TAB>
There should be many entries starting with group:, in the failing case there's only
one. Presumably because in group_ptr all the pointers point to the same location,
that contains the last group added. The completion mechanism then removes duplicates.
It is not enough to assign holders.back ().c_str () (after having pushed the string in
the vector), because when the vector gets reallocated it can now point to stale memory.
I think we have to do it in two pass, prepare the vector of std::string, and then get
pointers to the strings. Something like this:
commit f9cab673480425a130b73394c3a63d256eadf314
Author: Simon Marchi <simon.marchi@ericsson.com>
Date: Fri Oct 20 16:22:40 2017 -0400
Fix syscall group completion
diff --git a/gdb/break-catch-syscall.c b/gdb/break-catch-syscall.c
index 82d3e36..095284c 100644
--- a/gdb/break-catch-syscall.c
+++ b/gdb/break-catch-syscall.c
@@ -562,7 +562,6 @@ catch_syscall_completer (struct cmd_list_element *cmd,
struct gdbarch *gdbarch = get_current_arch ();
gdb::unique_xmalloc_ptr<const char *> group_list;
const char *prefix;
- int i;
/* Completion considers ':' to be a word separator, so we use this to
verify whether the previous word was a group prefix. If so, we
@@ -590,14 +589,11 @@ catch_syscall_completer (struct cmd_list_element *cmd,
std::vector<std::string> holders;
/* Append "group:" prefix to syscall groups. */
- for (i = 0; group_ptr[i] != NULL; i++)
- {
- std::string prefixed_group = string_printf ("group:%s",
- group_ptr[i]);
+ for (int i = 0; group_ptr[i] != NULL; i++)
+ holders.push_back (string_printf ("group:%s", group_ptr[i]));
- group_ptr[i] = prefixed_group.c_str ();
- holders.push_back (std::move (prefixed_group));
- }
+ for (int i = 0; group_ptr[i] != NULL; i++)
+ group_ptr[i] = holders[i].c_str ();
if (syscall_list != NULL)
complete_on_enum (tracker, syscall_list.get (), word, word);