[RFA,3/3] Remove cleanups from break-catch-syscall.c

Message ID 20171018040645.7212-3-tom@tromey.com
State New, archived
Headers

Commit Message

Tom Tromey Oct. 18, 2017, 4:06 a.m. UTC
  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(-)
  

Comments

Simon Marchi Oct. 19, 2017, 9 p.m. UTC | #1
On 2017-10-18 00:06, 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);

Maybe an opportunity to std::move?  Alternatively:

   holders.emplace_back (string_printf ("group:%s", group_ptr[i]));
   group_ptr[i] = holders.back ().c_str ();

LGTM in any case, as well as the two previous patches.

Simon
  
Tom Tromey Oct. 19, 2017, 9:57 p.m. UTC | #2
>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:

Simon> Maybe an opportunity to std::move?  Alternatively:
Simon>   holders.emplace_back (string_printf ("group:%s", group_ptr[i]));
Simon>   group_ptr[i] = holders.back ().c_str ();

I made this change.  Thanks for noticing this.

Tom
  
Tom Tromey Oct. 19, 2017, 9:57 p.m. UTC | #3
>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:

>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:
Simon> Maybe an opportunity to std::move?  Alternatively:
Simon> holders.emplace_back (string_printf ("group:%s", group_ptr[i]));
Simon> group_ptr[i] = holders.back ().c_str ();

Tom> I made this change.  Thanks for noticing this.

Oops, I meant to say, I made the std::move change.

Tom
  

Patch

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);
 	}
 
       if (syscall_list != NULL)
-	complete_on_enum (tracker, syscall_list, word, word);
+	complete_on_enum (tracker, syscall_list.get (), word, word);
       if (group_list != NULL)
-	complete_on_enum (tracker, group_list, word, word);
+	complete_on_enum (tracker, group_ptr, word, word);
     }
-
-  xfree (syscall_list);
-  xfree (group_list);
-  do_cleanups (cleanups);
 }
 
 static void