[RFA] C++-ify parse_format_string
Tom Tromey
tom@tromey.com
Thu Nov 23 16:46:00 GMT 2017
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-11-23 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-11-23 Tom Tromey <tom@tromey.com>
* ax.c (ax_printf): Update.
---
gdb/ChangeLog | 16 +++++++++++++
gdb/ax-gdb.c | 17 ++++----------
gdb/ax-gdb.h | 2 --
gdb/breakpoint.c | 19 ++++-----------
gdb/common/format.c | 62 ++++---------------------------------------------
gdb/common/format.h | 42 ++++++++++++++++++++++++++-------
gdb/gdbserver/ChangeLog | 4 ++++
gdb/gdbserver/ax.c | 22 ++++++++----------
gdb/printcmd.c | 41 +++++++++++---------------------
9 files changed, 89 insertions(+), 136 deletions(-)
diff --git a/gdb/ax-gdb.c b/gdb/ax-gdb.c
index 52ca081a82..e22e0e6582 100644
--- a/gdb/ax-gdb.c
+++ b/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));
@@ -2680,12 +2679,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
@@ -2704,9 +2699,7 @@ maint_agent_printf_command (const char *cmdrest, int from_tty)
format_start = cmdrest;
- fpieces = parse_format_string (&cmdrest);
-
- old_chain = make_cleanup (free_format_pieces_cleanup, &fpieces);
+ format_pieces fpieces (&cmdrest);
format_end = cmdrest;
@@ -2722,15 +2715,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 ();
- ++nargs;
+ argvec.push_back (expr.release ());
cmdrest = cmd1;
if (*cmdrest == ',')
++cmdrest;
@@ -2741,14 +2733,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 ();
}
diff --git a/gdb/ax-gdb.h b/gdb/ax-gdb.h
index 8b5ab46c66..834ddffe05 100644
--- a/gdb/ax-gdb.h
+++ b/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 */
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index b48c405b08..41c542e08b 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -2230,12 +2230,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)
@@ -2252,9 +2248,7 @@ parse_cmd_to_aexpr (CORE_ADDR scope, char *cmd)
format_start = cmdrest;
- fpieces = parse_format_string (&cmdrest);
-
- old_cleanups = make_cleanup (free_format_pieces_cleanup, &fpieces);
+ format_pieces fpieces (&cmdrest);
format_end = cmdrest;
@@ -2272,17 +2266,14 @@ parse_cmd_to_aexpr (CORE_ADDR scope, char *cmd)
/* For each argument, make an expression. */
- argvec = (struct expression **) alloca (strlen (cmd)
- * sizeof (struct expression *));
-
- nargs = 0;
+ std::vector<struct expression *> argvec;
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;
@@ -2296,7 +2287,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)
{
@@ -2306,8 +2297,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;
}
diff --git a/gdb/common/format.c b/gdb/common/format.c
index 8cb15511fa..95cb8053c4 100644
--- a/gdb/common/format.c
+++ b/gdb/common/format.c
@@ -20,17 +20,13 @@
#include "common-defs.h"
#include "format.h"
-struct format_piece *
-parse_format_string (const char **arg)
+format_pieces::format_pieces (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);
-
- max_pieces = strlen (string) + 2;
-
- pieces = XNEWVEC (struct format_piece, max_pieces);
-
- next_frag = 0;
+ m_storage.reset (current_substring);
/* 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;
- pieces[next_frag].argclass = literal_piece;
- next_frag++;
+ m_pieces.emplace_back (sub_start, literal_piece);
percent_loc = f - 1;
@@ -343,9 +332,7 @@ parse_format_string (const char **arg)
prev_start = f;
- pieces[next_frag].string = sub_start;
- pieces[next_frag].argclass = this_argclass;
- next_frag++;
+ m_pieces.emplace_back (sub_start, this_argclass);
}
/* 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;
- 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;
+ m_pieces.emplace_back (sub_start, literal_piece);
}
-
-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;
- }
-}
-
diff --git a/gdb/common/format.h b/gdb/common/format.h
index f3a94b8bbb..dd083f9ac1 100644
--- a/gdb/common/format.h
+++ b/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
- rewrite ARG with a pointer to the end of the format string. */
+class format_pieces
+{
+public:
+
+ format_pieces (const char **arg);
+ ~format_pieces () = default;
+
+ DISABLE_COPY_AND_ASSIGN (format_pieces);
-extern struct format_piece *parse_format_string (const char **arg);
+ format_piece &operator[] (size_t index)
+ {
+ return m_pieces[index];
+ }
-/* Given a pointer to an array of format pieces, free any memory that
- would have been allocated by parse_format_string. */
+ typedef std::vector<format_piece>::iterator iterator;
-extern void free_format_pieces (struct format_piece *frags);
+ iterator begin ()
+ {
+ return m_pieces.begin ();
+ }
-/* Freeing, cast as a cleanup. */
+ iterator end ()
+ {
+ return m_pieces.end ();
+ }
-extern void free_format_pieces_cleanup (void *);
+private:
+
+ std::vector<format_piece> m_pieces;
+ gdb::unique_xmalloc_ptr<char> m_storage;
+};
#endif /* COMMON_FORMAT_H */
diff --git a/gdb/gdbserver/ax.c b/gdb/gdbserver/ax.c
index 35ed2c69ad..7e5a409cfd 100644
--- a/gdb/gdbserver/ax.c
+++ b/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, fp;
- char *current_substring;
+ int i;
+ const 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++)
- if (fpieces[fp].argclass != literal_piece)
+ for (auto &&piece : fpieces)
+ 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);
- switch (fpieces[fp].argclass)
+ current_substring, piece.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);
}
diff --git a/gdb/printcmd.c b/gdb/printcmd.c
index 2e596d1f09..7ca86232a1 100644
--- a/gdb/printcmd.c
+++ b/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;
- int allocated_args = 20;
- struct cleanup *old_cleanups;
-
- val_args = XNEWVEC (struct value *, allocated_args);
- old_cleanups = make_cleanup (free_current_contents, &val_args);
+ std::vector<struct value *> 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);
-
- make_cleanup (free_format_pieces_cleanup, &fpieces);
+ format_pieces fpieces (&s);
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;
- char *current_substring;
+ int i;
+ const char *current_substring;
nargs_wanted = 0;
- for (fr = 0; fpieces[fr].string != NULL; fr++)
- if (fpieces[fr].argclass != literal_piece)
+ for (auto &&piece : fpieces)
+ 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;
- switch (fpieces[fr].argclass)
+ current_substring = piece.string;
+ 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. */
--
2.13.6
More information about the Gdb-patches
mailing list