[pushed] Make target_options_to_string return an std::string

[pushed] Make target_options_to_string return an std::string

[Simon Marchi]

Return an std::string instead of a char *, saving some manual freeing.

I only manually tested with "set debug target 1" and "set debug lin-lwp
1", since this only deals with debug output.

gdb/ChangeLog:

	* target.h (target_options_to_string): Return an std::string.
	* target.c (str_comma_list_concat_elem): Return void, use
	std::string.
	(do_option): Likewise.
	(target_options_to_string): Return an std::string.
	* linux-nat.c (linux_nat_target::wait): Adjust.
	* target-debug.h (target_debug_print_options): Adjust.
---
 gdb/ChangeLog      | 10 ++++++++++
 gdb/linux-nat.c    |  7 ++-----
 gdb/target-debug.h |  5 ++---
 gdb/target.c       | 36 +++++++++++++++---------------------
 gdb/target.h       |  5 ++---
 5 files changed, 31 insertions(+), 32 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 838029bf239e..6702b445b7c7 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,13 @@
+2018-08-07  Simon Marchi  <simon.marchi@polymtl.ca>
+
+	* target.h (target_options_to_string): Return an std::string.
+	* target.c (str_comma_list_concat_elem): Return void, use
+	std::string.
+	(do_option): Likewise.
+	(target_options_to_string): Return an std::string.
+	* linux-nat.c (linux_nat_target::wait): Adjust.
+	* target-debug.h (target_debug_print_options): Adjust.
+
 2018-08-07  Tom Tromey  <tom@tromey.com>
 
 	* Makefile.in (CPPFLAGS): New variable.
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index d2c88ad0cd42..64015e752079 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -3555,14 +3555,11 @@ linux_nat_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
 
   if (debug_linux_nat)
     {
-      char *options_string;
-
-      options_string = target_options_to_string (target_options);
+      std::string options_string = target_options_to_string (target_options);
       fprintf_unfiltered (gdb_stdlog,
 			  "linux_nat_wait: [%s], [%s]\n",
 			  target_pid_to_str (ptid),
-			  options_string);
-      xfree (options_string);
+			  options_string.c_str ());
     }
 
   /* Flush the async file first.  */
diff --git a/gdb/target-debug.h b/gdb/target-debug.h
index 331baf57534a..1e904b95d608 100644
--- a/gdb/target-debug.h
+++ b/gdb/target-debug.h
@@ -203,10 +203,9 @@ target_debug_print_struct_target_waitstatus_p (struct target_waitstatus *status)
 static void
 target_debug_print_options (int options)
 {
-  char *str = target_options_to_string (options);
+  std::string str = target_options_to_string (options);
 
-  fputs_unfiltered (str, gdb_stdlog);
-  xfree (str);
+  fputs_unfiltered (str.c_str (), gdb_stdlog);
 }
 
 static void
diff --git a/gdb/target.c b/gdb/target.c
index eba07cc91742..a5245abb2811 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -3441,51 +3441,45 @@ target_continue (ptid_t ptid, enum gdb_signal signal)
   target_resume (ptid, 0, signal);
 }
 
-/* Concatenate ELEM to LIST, a comma separate list, and return the
-   result.  The LIST incoming argument is released.  */
+/* Concatenate ELEM to LIST, a comma separate list.  */
 
-static char *
-str_comma_list_concat_elem (char *list, const char *elem)
+static void
+str_comma_list_concat_elem (std::string *list, const char *elem)
 {
-  if (list == NULL)
-    return xstrdup (elem);
-  else
-    return reconcat (list, list, ", ", elem, (char *) NULL);
+  if (!list->empty ())
+    list->append (", ");
+
+  list->append (elem);
 }
 
 /* Helper for target_options_to_string.  If OPT is present in
    TARGET_OPTIONS, append the OPT_STR (string version of OPT) in RET.
-   Returns the new resulting string.  OPT is removed from
-   TARGET_OPTIONS.  */
+   OPT is removed from TARGET_OPTIONS.  */
 
-static char *
-do_option (int *target_options, char *ret,
+static void
+do_option (int *target_options, std::string *ret,
 	   int opt, const char *opt_str)
 {
   if ((*target_options & opt) != 0)
     {
-      ret = str_comma_list_concat_elem (ret, opt_str);
+      str_comma_list_concat_elem (ret, opt_str);
       *target_options &= ~opt;
     }
-
-  return ret;
 }
 
-char *
+std::string
 target_options_to_string (int target_options)
 {
-  char *ret = NULL;
+  std::string ret;
 
 #define DO_TARG_OPTION(OPT) \
-  ret = do_option (&target_options, ret, OPT, #OPT)
+  do_option (&target_options, &ret, OPT, #OPT)
 
   DO_TARG_OPTION (TARGET_WNOHANG);
 
   if (target_options != 0)
-    ret = str_comma_list_concat_elem (ret, "unknown???");
+    str_comma_list_concat_elem (&ret, "unknown???");
 
-  if (ret == NULL)
-    ret = xstrdup ("");
   return ret;
 }
 
diff --git a/gdb/target.h b/gdb/target.h
index 18c4a84ca5b5..39aa8c3c730a 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -116,9 +116,8 @@ struct syscall
     const char *name;
   };
 
-/* Return a pretty printed form of TARGET_OPTIONS.
-   Space for the result is malloc'd, caller must free.  */
-extern char *target_options_to_string (int target_options);
+/* Return a pretty printed form of TARGET_OPTIONS.  */
+extern std::string target_options_to_string (int target_options);
 
 /* Possible types of events that the inferior handler will have to
    deal with.  */
-- 
2.18.0

Re: [pushed] Make target_options_to_string return an std::string

[Tom Tromey]

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

Simon> Return an std::string instead of a char *, saving some manual freeing.
Simon> I only manually tested with "set debug target 1" and "set debug lin-lwp
Simon> 1", since this only deals with debug output.

Simon> -/* Concatenate ELEM to LIST, a comma separate list, and return the
Simon> -   result.  The LIST incoming argument is released.  */
Simon> +/* Concatenate ELEM to LIST, a comma separate list.  */

I think this should read "comma-separated".

Other than that nit, this looks good to me.

Tom

Re: [pushed] Make target_options_to_string return an std::string

[Simon Marchi]

On 2018-08-08 11:11 AM, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:
> 
> Simon> Return an std::string instead of a char *, saving some manual freeing.
> Simon> I only manually tested with "set debug target 1" and "set debug lin-lwp
> Simon> 1", since this only deals with debug output.
> 
> Simon> -/* Concatenate ELEM to LIST, a comma separate list, and return the
> Simon> -   result.  The LIST incoming argument is released.  */
> Simon> +/* Concatenate ELEM to LIST, a comma separate list.  */
> 
> I think this should read "comma-separated".
> 
> Other than that nit, this looks good to me.

Good catch, here's what I pushed:

From fdbac7d8d1c9403a857db6e0c1dc92ce7bb65925 Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@ericsson.com>
Date: Wed, 8 Aug 2018 18:13:18 -0400
Subject: [PATCH] Fix some comments in target.c

Fix a typo and add a missing one.

gdb/ChangeLog:

	* target.c (str_comma_list_concat_elem): Fix typo in comment.
	(target_options_to_string): Add comment.
---
 gdb/ChangeLog | 5 +++++
 gdb/target.c  | 4 +++-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 49a7e39..4d0593f 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2018-08-08  Simon Marchi  <simon.marchi@ericsson.com>
+
+	* target.c (str_comma_list_concat_elem): Fix typo in comment.
+	(target_options_to_string): Add comment.
+
 2018-08-08  Tom Tromey  <tom@tromey.com>

 	* unittests/scoped_mmap-selftests.c: Check result of "write".
diff --git a/gdb/target.c b/gdb/target.c
index 115e9ae..2d98954 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -3447,7 +3447,7 @@ target_continue (ptid_t ptid, enum gdb_signal signal)
   target_resume (ptid, 0, signal);
 }

-/* Concatenate ELEM to LIST, a comma separate list.  */
+/* Concatenate ELEM to LIST, a comma-separated list.  */

 static void
 str_comma_list_concat_elem (std::string *list, const char *elem)
@@ -3473,6 +3473,8 @@ do_option (int *target_options, std::string *ret,
     }
 }

+/* See target.h.  */
+
 std::string
 target_options_to_string (int target_options)
 {
-- 
2.7.4