Fix tracepoint.c:parse_tracepoint_definition leak (and one more)

Coverity points out that gdb/tracepoint.c:parse_tracepoint_definition
can leak 'cond' in this line:

      cond = (char *) xmalloc (2 * xlen + 1);

That can leak because we're in a loop and 'cond' may have already been
xmalloc'ed into in a previous iteration.  That won't normally happen,
because we don't expect to see a tracepoint definition with multiple
conditions listed, but, it doesn't hurt to be pedantically correct,
in case some stub manages to send something odd back to GDB.

At first I thought I'd just replace the xmalloc call with:

      cond = (char *) xrealloc (cond, 2 * xlen + 1);

and be done with it.  However, my pedantic self realizes that
warning() can throw as well (due to pagination + Ctrl-C), so I fixed
it using gdb::unique_xmalloc_ptr instead.

While doing this, I noticed that these vectors in struct uploaded_tp:

  std::vector<char *> actions;
  std::vector<char *> step_actions;

hold heap-allocated strings, but nothing is freeing the strings,
AFAICS.

So I ended up switching all the heap-allocated strings in uploaded_tp
to unique pointers.  This patch is the result of that.

I also wrote an alternative, but similar patch that uses std::string
throughout instead of gdb::unique_xmalloc_ptr, but in the end reverted
it because the code didn't look that much better, and I kind of
dislike replacing pointers with fat std::string's (3 or 4 times the
size of a pointer) in structures.

gdb/ChangeLog:
2019-01-10  Pedro Alves  <palves@redhat.com>

	* breakpoint.c (read_uploaded_action)
	(create_tracepoint_from_upload): Adjust to use
	gdb::unique_xmalloc_ptr.
	* ctf.c (ctf_write_uploaded_tp):
	(SET_ARRAY_FIELD): Use emplace_back.
	(SET_STRING_FIELD): Adjust to use gdb::unique_xmalloc_ptr.
	* tracefile-tfile.c (tfile_write_uploaded_tp):
	* tracepoint.c (parse_tracepoint_definition): Adjust to use
	gdb::unique_xmalloc_ptr.
	* tracepoint.h (struct uploaded_tp) <cond, actions, step_actions,
	at_string, cond_string, cmd_strings>: Replace char pointers
	with gdb::unique_xmalloc_ptr.

1 files changed, 17 insertions, 13 deletions

diff --git a/gdb/ctf.c b/gdb/ctf.c
index d99e1cd..7a95df7 100644
--- a/gdb/ctf.c
+++ b/gdb/ctf.c
@@ -596,38 +596,42 @@ ctf_write_uploaded_tp (struct trace_file_writer *self,
 
   /* condition  */
   if (tp->cond != NULL)
-    ctf_save_write (&writer->tcs, (gdb_byte *) tp->cond, strlen (tp->cond));
+    ctf_save_write (&writer->tcs, (gdb_byte *) tp->cond.get (),
+		    strlen (tp->cond.get ()));
   ctf_save_write (&writer->tcs, &zero, 1);
 
   /* actions */
   u32 = tp->actions.size ();
   ctf_save_align_write (&writer->tcs, (gdb_byte *) &u32, 4, 4);
-  for (char *act : tp->actions)
-    ctf_save_write (&writer->tcs, (gdb_byte *) act, strlen (act) + 1);
+  for (const auto &act : tp->actions)
+    ctf_save_write (&writer->tcs, (gdb_byte *) act.get (),
+		    strlen (act.get ()) + 1);
 
   /* step_actions */
   u32 = tp->step_actions.size ();
   ctf_save_align_write (&writer->tcs, (gdb_byte *) &u32, 4, 4);
-  for (char *act : tp->step_actions)
-    ctf_save_write (&writer->tcs, (gdb_byte *) act, strlen (act) + 1);
+  for (const auto &act : tp->step_actions)
+    ctf_save_write (&writer->tcs, (gdb_byte *) act.get (),
+		    strlen (act.get ()) + 1);
 
   /* at_string */
   if (tp->at_string != NULL)
-    ctf_save_write (&writer->tcs, (gdb_byte *) tp->at_string,
-		    strlen (tp->at_string));
+    ctf_save_write (&writer->tcs, (gdb_byte *) tp->at_string.get (),
+		    strlen (tp->at_string.get ()));
   ctf_save_write (&writer->tcs, &zero, 1);
 
   /* cond_string */
   if (tp->cond_string != NULL)
-    ctf_save_write (&writer->tcs, (gdb_byte *) tp->cond_string,
-		    strlen (tp->cond_string));
+    ctf_save_write (&writer->tcs, (gdb_byte *) tp->cond_string.get (),
+		    strlen (tp->cond_string.get ()));
   ctf_save_write (&writer->tcs, &zero, 1);
 
   /* cmd_strings */
   u32 = tp->cmd_strings.size ();
   ctf_save_align_write (&writer->tcs, (gdb_byte *) &u32, 4, 4);
-  for (char *act : tp->cmd_strings)
-    ctf_save_write (&writer->tcs, (gdb_byte *) act, strlen (act) + 1);
+  for (const auto &act : tp->cmd_strings)
+    ctf_save_write (&writer->tcs, (gdb_byte *) act.get (),
+		    strlen (act.get ()) + 1);
 
 }
 
@@ -1023,7 +1027,7 @@ ctf_read_tsv (struct uploaded_tsv **uploaded_tsvs)
 	  const struct bt_definition *element				\
 	    = bt_ctf_get_index ((EVENT), def, i);			\
 									\
-	  (VAR)->ARRAY.push_back					\
+	  (VAR)->ARRAY.emplace_back					\
 	    (xstrdup (bt_ctf_get_string (element)));			\
 	}								\
     }									\
@@ -1040,7 +1044,7 @@ ctf_read_tsv (struct uploaded_tsv **uploaded_tsvs)
 							   #FIELD));	\
 									\
       if (strlen (p) > 0)						\
-	(VAR)->FIELD = xstrdup (p);					\
+	(VAR)->FIELD.reset (xstrdup (p));				\
       else								\
 	(VAR)->FIELD = NULL;						\
     }									\