C++-ify parse_format_string

This replaces parse_format_string with a class, removing some
constructors along the way.  While doing this, I found that one
argument to gen_printf is unused, so I removed it.

Also, I am not completely sure, but the use of `release' in
maint_agent_printf_command and parse_cmd_to_aexpr seems like it may
leak expressions.

Regression tested by the buildbot.

ChangeLog
2017-12-08  Tom Tromey  <tom@tromey.com>

	* printcmd.c (ui_printf): Update.  Use std::vector.
	* common/format.h (struct format_piece): Add constructor.
	<string>: Now const.
	(class format_pieces): New class.
	(parse_format_string, free_format_pieces)
	(free_format_pieces_cleanup): Remove.
	* common/format.c (format_pieces::format_pieces): Rename from
	parse_format_string.  Update.
	(free_format_pieces, free_format_pieces_cleanup): Remove.
	* breakpoint.c (parse_cmd_to_aexpr): Update.  Use std::vector.
	* ax-gdb.h (gen_printf): Remove argument.
	* ax-gdb.c (gen_printf): Remove "frags" argument.
	(maint_agent_printf_command): Update.  Use std::vector.

gdbserver/ChangeLog
2017-12-08  Tom Tromey  <tom@tromey.com>

	* ax.c (ax_printf): Update.

gdb/ChangeLog

@@ -1,3 +1,19 @@
+2017-12-08  Tom Tromey  <tom@tromey.com>
+
+	* printcmd.c (ui_printf): Update.  Use std::vector.
+	* common/format.h (struct format_piece): Add constructor.
+	<string>: Now const.
+	(class format_pieces): New class.
+	(parse_format_string, free_format_pieces)
+	(free_format_pieces_cleanup): Remove.
+	* common/format.c (format_pieces::format_pieces): Rename from
+	parse_format_string.  Update.
+	(free_format_pieces, free_format_pieces_cleanup): Remove.
+	* breakpoint.c (parse_cmd_to_aexpr): Update.  Use std::vector.
+	* ax-gdb.h (gen_printf): Remove argument.
+	* ax-gdb.c (gen_printf): Remove "frags" argument.
+	(maint_agent_printf_command): Update.  Use std::vector.
+
 2017-12-08  Yao Qi  <yao.qi@linaro.org>
 
 	PR breakpionts/22567

gdb/ax-gdb.c

@@ -2541,7 +2541,6 @@ agent_expr_up
 gen_printf (CORE_ADDR scope, struct gdbarch *gdbarch,
 	    CORE_ADDR function, LONGEST channel,
 	    const char *format, int fmtlen,
-	    struct format_piece *frags,
 	    int nargs, struct expression **exprs)
 {
   agent_expr_up ax (new agent_expr (gdbarch, scope));
@@ -2681,12 +2680,8 @@ agent_eval_command (const char *exp, int from_tty)
 static void
 maint_agent_printf_command (const char *cmdrest, int from_tty)
 {
-  struct cleanup *old_chain = 0;
-  struct expression *argvec[100];
   struct frame_info *fi = get_current_frame ();	/* need current scope */
   const char *format_start, *format_end;
-  struct format_piece *fpieces;
-  int nargs;
 
   /* We don't deal with overlay debugging at the moment.  We need to
      think more carefully about this.  If you copy this code into
@@ -2705,9 +2700,7 @@ maint_agent_printf_command (const char *cmdrest, int from_tty)
 
   format_start = cmdrest;
 
-  fpieces = parse_format_string (&cmdrest);
+  format_pieces fpieces (&cmdrest);
-
-  old_chain = make_cleanup (free_format_pieces_cleanup, &fpieces);
 
   format_end = cmdrest;
 
@@ -2723,15 +2716,14 @@ maint_agent_printf_command (const char *cmdrest, int from_tty)
     cmdrest++;
   cmdrest = skip_spaces (cmdrest);
 
-  nargs = 0;
+  std::vector<struct expression *> argvec;
   while (*cmdrest != '\0')
     {
       const char *cmd1;
 
       cmd1 = cmdrest;
       expression_up expr = parse_exp_1 (&cmd1, 0, (struct block *) 0, 1);
-      argvec[nargs] = expr.release ();
+      argvec.push_back (expr.release ());
-      ++nargs;
       cmdrest = cmd1;
       if (*cmdrest == ',')
 	++cmdrest;
@@ -2742,14 +2734,13 @@ maint_agent_printf_command (const char *cmdrest, int from_tty)
   agent_expr_up agent = gen_printf (get_frame_pc (fi), get_current_arch (),
 				    0, 0,
 				    format_start, format_end - format_start,
-				    fpieces, nargs, argvec);
+				    argvec.size (), argvec.data ());
   ax_reqs (agent.get ());
   ax_print (gdb_stdout, agent.get ());
 
   /* It would be nice to call ax_reqs here to gather some general info
      about the expression, and then print out the result.  */
 
-  do_cleanups (old_chain);
   dont_repeat ();
 }
 

gdb/ax-gdb.h

@@ -120,10 +120,8 @@ extern void gen_expr (struct expression *exp, union exp_element **pc,
 
 extern void require_rvalue (struct agent_expr *ax, struct axs_value *value);
 
-struct format_piece;
 extern agent_expr_up gen_printf (CORE_ADDR, struct gdbarch *,
 				 CORE_ADDR, LONGEST, const char *, int,
-				 struct format_piece *,
 				 int, struct expression **);
 
 #endif /* AX_GDB_H */

gdb/breakpoint.c

@@ -2226,12 +2226,8 @@ build_target_condition_list (struct bp_location *bl)
 static agent_expr_up
 parse_cmd_to_aexpr (CORE_ADDR scope, char *cmd)
 {
-  struct cleanup *old_cleanups = 0;
-  struct expression **argvec;
   const char *cmdrest;
   const char *format_start, *format_end;
-  struct format_piece *fpieces;
-  int nargs;
   struct gdbarch *gdbarch = get_current_arch ();
 
   if (cmd == NULL)
@@ -2248,9 +2244,7 @@ parse_cmd_to_aexpr (CORE_ADDR scope, char *cmd)
 
   format_start = cmdrest;
 
-  fpieces = parse_format_string (&cmdrest);
+  format_pieces fpieces (&cmdrest);
-
-  old_cleanups = make_cleanup (free_format_pieces_cleanup, &fpieces);
 
   format_end = cmdrest;
 
@@ -2268,17 +2262,14 @@ parse_cmd_to_aexpr (CORE_ADDR scope, char *cmd)
 
   /* For each argument, make an expression.  */
 
-  argvec = (struct expression **) alloca (strlen (cmd)
+  std::vector<struct expression *> argvec;
-					 * sizeof (struct expression *));
-
-  nargs = 0;
   while (*cmdrest != '\0')
     {
       const char *cmd1;
 
       cmd1 = cmdrest;
       expression_up expr = parse_exp_1 (&cmd1, scope, block_for_pc (scope), 1);
-      argvec[nargs++] = expr.release ();
+      argvec.push_back (expr.release ());
       cmdrest = cmd1;
       if (*cmdrest == ',')
 	++cmdrest;
@@ -2292,7 +2283,7 @@ parse_cmd_to_aexpr (CORE_ADDR scope, char *cmd)
     {
       aexpr = gen_printf (scope, gdbarch, 0, 0,
 			  format_start, format_end - format_start,
-			  fpieces, nargs, argvec);
+			  argvec.size (), argvec.data ());
     }
   CATCH (ex, RETURN_MASK_ERROR)
     {
@@ -2302,8 +2293,6 @@ parse_cmd_to_aexpr (CORE_ADDR scope, char *cmd)
     }
   END_CATCH
 
-  do_cleanups (old_cleanups);
-
   /* We have a valid agent expression, return it.  */
   return aexpr;
 }

gdb/common/format.c

@@ -20,17 +20,13 @@
 #include "common-defs.h"
 #include "format.h"
 
-struct format_piece *
+format_pieces::format_pieces (const char **arg)
-parse_format_string (const char **arg)
 {
   const char *s;
   char *f, *string;
   const char *prev_start;
   const char *percent_loc;
   char *sub_start, *current_substring;
-  struct format_piece *pieces;
-  int next_frag;
-  int max_pieces;
   enum argclass this_argclass;
 
   s = *arg;
@@ -100,12 +96,7 @@ parse_format_string (const char **arg)
   /* Need extra space for the '\0's.  Doubling the size is sufficient.  */
 
   current_substring = (char *) xmalloc (strlen (string) * 2 + 1000);
-
+  m_storage.reset (current_substring);
-  max_pieces = strlen (string) + 2;
-
-  pieces = XNEWVEC (struct format_piece, max_pieces);
-
-  next_frag = 0;
 
   /* Now scan the string for %-specs and see what kinds of args they want.
      argclass classifies the %-specs so we can give printf-type functions
@@ -135,9 +126,7 @@ parse_format_string (const char **arg)
 	current_substring += f - 1 - prev_start;
 	*current_substring++ = '\0';
 
-	pieces[next_frag].string = sub_start;
+	m_pieces.emplace_back (sub_start, literal_piece);
-	pieces[next_frag].argclass = literal_piece;
-	next_frag++;
 
 	percent_loc = f - 1;
 
@@ -343,9 +332,7 @@ parse_format_string (const char **arg)
 
 	prev_start = f;
 
-	pieces[next_frag].string = sub_start;
+	m_pieces.emplace_back (sub_start, this_argclass);
-	pieces[next_frag].argclass = this_argclass;
-	next_frag++;
       }
 
   /* Record the remainder of the string.  */
@@ -356,44 +343,5 @@ parse_format_string (const char **arg)
   current_substring += f - prev_start;
   *current_substring++ = '\0';
 
-  pieces[next_frag].string = sub_start;
+  m_pieces.emplace_back (sub_start, literal_piece);
-  pieces[next_frag].argclass = literal_piece;
-  next_frag++;
-
-  /* Record an end-of-array marker.  */
-
-  pieces[next_frag].string = NULL;
-  pieces[next_frag].argclass = literal_piece;
-
-  return pieces;
 }
-
-void
-free_format_pieces (struct format_piece *pieces)
-{
-  if (!pieces)
-    return;
-
-  /* We happen to know that all the string pieces are in the block
-     pointed to by the first string piece.  */
-  if (pieces[0].string)
-    xfree (pieces[0].string);
-
-  xfree (pieces);
-}
-
-void
-free_format_pieces_cleanup (void *ptr)
-{
-  struct format_piece **location = (struct format_piece **) ptr;
-
-  if (location == NULL)
-    return;
-
-  if (*location != NULL)
-    {
-      free_format_pieces (*location);
-      *location = NULL;
-    }
-}
-

gdb/common/format.h

@@ -48,22 +48,46 @@ enum argclass
 
 struct format_piece
 {
-  char *string;
+  format_piece (const char *str, enum argclass argc)
+    : string (str),
+      argclass (argc)
+  {
+  }
+
+  const char *string;
   enum argclass argclass;
 };
 
-/* Return an array of printf fragments found at the given string, and
+class format_pieces
-   rewrite ARG with a pointer to the end of the format string.  */
+{
+public:
 
-extern struct format_piece *parse_format_string (const char **arg);
+  format_pieces (const char **arg);
+  ~format_pieces () = default;
 
-/* Given a pointer to an array of format pieces, free any memory that
+  DISABLE_COPY_AND_ASSIGN (format_pieces);
-   would have been allocated by parse_format_string.  */
 
-extern void free_format_pieces (struct format_piece *frags);
+  format_piece &operator[] (size_t index)
+  {
+    return m_pieces[index];
+  }
 
-/* Freeing, cast as a cleanup.  */
+  typedef std::vector<format_piece>::iterator iterator;
 
-extern void free_format_pieces_cleanup (void *);
+  iterator begin ()
+  {
+    return m_pieces.begin ();
+  }
+
+  iterator end ()
+  {
+    return m_pieces.end ();
+  }
+
+private:
+
+  std::vector<format_piece> m_pieces;
+  gdb::unique_xmalloc_ptr<char> m_storage;
+};
 
 #endif /* COMMON_FORMAT_H */

gdb/gdbserver/ChangeLog

@@ -1,3 +1,7 @@
+2017-12-08  Tom Tromey  <tom@tromey.com>
+
+	* ax.c (ax_printf): Update.
+
 2017-12-07  Yao Qi  <yao.qi@linaro.org>
 
 	* linux-aarch64-ipa.c (initialize_low_tracepoint): Call

gdb/gdbserver/ax.c

@@ -816,30 +816,29 @@ ax_printf (CORE_ADDR fn, CORE_ADDR chan, const char *format,
 	   int nargs, ULONGEST *args)
 {
   const char *f = format;
-  struct format_piece *fpieces;
+  int i;
-  int i, fp;
+  const char *current_substring;
-  char *current_substring;
   int nargs_wanted;
 
   ax_debug ("Printf of \"%s\" with %d args", format, nargs);
 
-  fpieces = parse_format_string (&f);
+  format_pieces fpieces (&f);
 
   nargs_wanted = 0;
-  for (fp = 0; fpieces[fp].string != NULL; fp++)
+  for (auto &&piece : fpieces)
-    if (fpieces[fp].argclass != literal_piece)
+    if (piece.argclass != literal_piece)
       ++nargs_wanted;
 
   if (nargs != nargs_wanted)
     error (_("Wrong number of arguments for specified format-string"));
 
   i = 0;
-  for (fp = 0; fpieces[fp].string != NULL; fp++)
+  for (auto &&piece : fpieces)
     {
-      current_substring = fpieces[fp].string;
+      current_substring = piece.string;
       ax_debug ("current substring is '%s', class is %d",
-		current_substring, fpieces[fp].argclass);
+		current_substring, piece.argclass);
-      switch (fpieces[fp].argclass)
+      switch (piece.argclass)
 	{
 	case string_arg:
 	  {
@@ -914,11 +913,10 @@ ax_printf (CORE_ADDR fn, CORE_ADDR chan, const char *format,
 	}
 
       /* Maybe advance to the next argument.  */
-      if (fpieces[fp].argclass != literal_piece)
+      if (piece.argclass != literal_piece)
 	++i;
     }
 
-  free_format_pieces (fpieces);
   fflush (stdout);
 }
 

gdb/printcmd.c

@@ -2427,14 +2427,8 @@ printf_pointer (struct ui_file *stream, const char *format,
 static void
 ui_printf (const char *arg, struct ui_file *stream)
 {
-  struct format_piece *fpieces;
   const char *s = arg;
-  struct value **val_args;
+  std::vector<struct value *> val_args;
-  int allocated_args = 20;
-  struct cleanup *old_cleanups;
-
-  val_args = XNEWVEC (struct value *, allocated_args);
-  old_cleanups = make_cleanup (free_current_contents, &val_args);
 
   if (s == 0)
     error_no_arg (_("format-control string and values to print"));
@@ -2445,9 +2439,7 @@ ui_printf (const char *arg, struct ui_file *stream)
   if (*s++ != '"')
     error (_("Bad format string, missing '\"'."));
 
-  fpieces = parse_format_string (&s);
+  format_pieces fpieces (&s);
-
-  make_cleanup (free_format_pieces_cleanup, &fpieces);
 
   if (*s++ != '"')
     error (_("Bad format string, non-terminated '\"'."));
@@ -2462,14 +2454,13 @@ ui_printf (const char *arg, struct ui_file *stream)
   s = skip_spaces (s);
 
   {
-    int nargs = 0;
     int nargs_wanted;
-    int i, fr;
+    int i;
-    char *current_substring;
+    const char *current_substring;
 
     nargs_wanted = 0;
-    for (fr = 0; fpieces[fr].string != NULL; fr++)
+    for (auto &&piece : fpieces)
-      if (fpieces[fr].argclass != literal_piece)
+      if (piece.argclass != literal_piece)
 	++nargs_wanted;
 
     /* Now, parse all arguments and evaluate them.
@@ -2479,28 +2470,23 @@ ui_printf (const char *arg, struct ui_file *stream)
       {
 	const char *s1;
 
-	if (nargs == allocated_args)
-	  val_args = (struct value **) xrealloc ((char *) val_args,
-						 (allocated_args *= 2)
-						 * sizeof (struct value *));
 	s1 = s;
-	val_args[nargs] = parse_to_comma_and_eval (&s1);
+	val_args.push_back (parse_to_comma_and_eval (&s1));
 
-	nargs++;
 	s = s1;
 	if (*s == ',')
 	  s++;
       }
 
-    if (nargs != nargs_wanted)
+    if (val_args.size () != nargs_wanted)
       error (_("Wrong number of arguments for specified format-string"));
 
     /* Now actually print them.  */
     i = 0;
-    for (fr = 0; fpieces[fr].string != NULL; fr++)
+    for (auto &&piece : fpieces)
       {
-	current_substring = fpieces[fr].string;
+	current_substring = piece.string;
-	switch (fpieces[fr].argclass)
+	switch (piece.argclass)
 	  {
 	  case string_arg:
 	    printf_c_string (stream, current_substring, val_args[i]);
@@ -2569,7 +2555,7 @@ ui_printf (const char *arg, struct ui_file *stream)
 	  case dec64float_arg:
 	  case dec128float_arg:
 	    printf_floating (stream, current_substring, val_args[i],
-			     fpieces[fr].argclass);
+			     piece.argclass);
 	    break;
 	  case ptr_arg:
 	    printf_pointer (stream, current_substring, val_args[i]);
@@ -2590,11 +2576,10 @@ ui_printf (const char *arg, struct ui_file *stream)
 			    _("failed internal consistency check"));
 	  }
 	/* Maybe advance to the next argument.  */
-	if (fpieces[fr].argclass != literal_piece)
+	if (piece.argclass != literal_piece)
 	  ++i;
       }
   }
-  do_cleanups (old_cleanups);
 }
 
 /* Implement the "printf" command.  */