[RFA] C++-ify parse_format_string
Commit Message
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(-)
Comments
On 2017-11-23 11:46 AM, Tom Tromey wrote:
> 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.
It looks fishy indeed. You could change argvec to be a vector of
expression_up.
> Regression tested by the buildbot.
I have a patch in some branch that does essentially the same thing, so
I was able to compare our approaches. In my version, I removed the
big allocation that is shared among pieces, and made each piece have
its own std::string. Unless we want to keep the current allocation
scheme for performance/memory usage reasons, I think that using
std::strings simplifies things in the parse_format_string function.
The format_pieces structure is replaced with an std::vector of
format_piece.
I rebased it and stole some parts from your patch for other little cleanups
(e.g. remove unused argument, use vectors of expressions). Here it is,
the 3rd from the top:
https://github.com/simark/binutils-gdb/commits/vec-format_piece
Let me know what you think about the approach, if you think it's good I'll
get it in a mergeable state.
Simon
On 11/23/2017 09:13 PM, Simon Marchi wrote:
> I have a patch in some branch that does essentially the same thing, so
> I was able to compare our approaches. In my version, I removed the
> big allocation that is shared among pieces, and made each piece have
> its own std::string. Unless we want to keep the current allocation
> scheme for performance/memory usage reasons, I think that using
> std::strings simplifies things in the parse_format_string function.
> The format_pieces structure is replaced with an std::vector of
> format_piece.
Sounds like a step backwards to me. If it simplifies things, then
it sounds like it might be because we're missing some utility,
like string_view or something like that.
Thanks,
Pedro Alves
On 2017-11-23 05:40 PM, Pedro Alves wrote:
> On 11/23/2017 09:13 PM, Simon Marchi wrote:
>
>> I have a patch in some branch that does essentially the same thing, so
>> I was able to compare our approaches. In my version, I removed the
>> big allocation that is shared among pieces, and made each piece have
>> its own std::string. Unless we want to keep the current allocation
>> scheme for performance/memory usage reasons, I think that using
>> std::strings simplifies things in the parse_format_string function.
>> The format_pieces structure is replaced with an std::vector of
>> format_piece.
>
> Sounds like a step backwards to me. If it simplifies things, then
> it sounds like it might be because we're missing some utility,
> like string_view or something like that.
I am not sure I understand, can you expand a little bit about what you
have in mind?
What I meant is that is changes things like this:
strncpy (current_substring, percent_loc, length_before_ll);
strcpy (current_substring + length_before_ll, "I64");
current_substring[length_before_ll + 3] =
percent_loc[length_before_ll + lcount];
current_substring += length_before_ll + 4;
into this
piece_string.assign (percent_loc, length_before_ll);
piece_string += "I64";
piece_string += percent_loc[length_before_ll + lcount];
Less magical number, less playing with offsets, etc.
Simon
On 11/24/2017 03:17 AM, Simon Marchi wrote:
> On 2017-11-23 05:40 PM, Pedro Alves wrote:
>> On 11/23/2017 09:13 PM, Simon Marchi wrote:
>>
>>> I have a patch in some branch that does essentially the same thing, so
>>> I was able to compare our approaches. In my version, I removed the
>>> big allocation that is shared among pieces, and made each piece have
>>> its own std::string. Unless we want to keep the current allocation
>>> scheme for performance/memory usage reasons, I think that using
>>> std::strings simplifies things in the parse_format_string function.
>>> The format_pieces structure is replaced with an std::vector of
>>> format_piece.
>>
>> Sounds like a step backwards to me. If it simplifies things, then
>> it sounds like it might be because we're missing some utility,
>> like string_view or something like that.
>
> I am not sure I understand, can you expand a little bit about what you
> have in mind?
I call it a step backwards because simplification is done at the
cost of efficiency (individual heap allocations, changing the
ownership model), when the original code avoided that. It's well known that
standard C++ is missing basic string manipulation utilities, but that's
no reason to force ourselves to consider std::string the ultimate tool, IMO.
There's C++ the library, and then there's the C++ the language. We can't
do anything about the latter, but we can extend the former. Later revisions
of the standard are adding more facilities, like std::string_view in C++17, and
now things like std::array_view / std::span (from gsl::span) have a very
good chance of making their way into C++20. Many C++ projects end up
reinventing something like those types, hence the attempts at
standardization, making those types lingua franca for non-owning use cases.
>
> What I meant is that is changes things like this:
>
> strncpy (current_substring, percent_loc, length_before_ll);
> strcpy (current_substring + length_before_ll, "I64");
> current_substring[length_before_ll + 3] =
> percent_loc[length_before_ll + lcount];
> current_substring += length_before_ll + 4;
>
> into this
>
> piece_string.assign (percent_loc, length_before_ll);
> piece_string += "I64";
> piece_string += percent_loc[length_before_ll + lcount];
>
> Less magical number, less playing with offsets, etc.
I meant that you don't need for each piece_string to own its
own deep copy of the string to get rid of the magical numbers.
You can get that even with a couple C-like free-functions, like:
// memcpy, null-terminate, and return advanced pointer.
char *append (char *p, const char *str);
char *append (char *p, const char *str, size_t len);
and then:
char *p = current_substring;
p = append (p, percent_loc, length_before_ll);
p = append (p, "I64");
p = append (p, percent_loc[length_before_ll + lcount]);
current_substring = p;
You could wrap that in a class, like:
/* Helper to build an null-terminated C string. */
struct zstring_builder
{
/* POS is a pointer to the position in buffer to work on. */
explicit zstring_builder (char *pos)
: m_pos (pos)
{}
// memcpy, advance m_pos, null-terminate.
zstring_builder &append (const char *s, size_type count);
zstring_builder &append (const char* s);
char *pos () { return m_pos; }
private:
char *m_pos;
};
Or:
/* Helper to build a null-terminated C string on top
of a vector. */
struct zstring_builder
{
/* POS is a pointer to the position in buffer to work on. */
explicit zstring_builder (std::vector<char> &buf, size_t pos);
// memcpy, advance m_pos, null-terminate.
zstring_builder &append (const char *s, size_type count);
zstring_builder &append (const char* s);
char *pos () { return &m_vector[m_pos]; }
private:
std::vector &m_buf;
size_t m_pos;
};
or something else along those lines, if you now make piece_string
a zstring_builder, you can get similar, and just as readable code:
zstring_builder piece_string (current_substring);
piece_string.append (percent_loc, length_before_ll);
piece_string.append ("I64");
piece_string.append (percent_loc[length_before_ll + lcount]);
current_substring = piece_string.pos ();
Alternatively, we could have something like a zstring_ref
class [1], as a non-owning mutable view of a C string (that never
reallocates), guaranteed to be null-terminated, and then the code
you shown would be just exactly the same. Open choices could be whether
to save the string size inside zstring_ref (like string_view), or
not saving a size, making it a really-thin wrapper around a
pointer instead.
std::string is great as an _owning_ SSO-enabled (which is sometimes a
curse when embedded in structures...) container of characters. But
when we don't want an owning container, the right tool / refactor can
still make the code just as readable.
[1] - I've thought several times because that something like a zstring_view
(like string_view, but guarantees null-termination) would be handy in
a lot of places. string_view isn't mutable, though, hence the
alternative name instead. could be mut_string_view too, for example.
Thanks,
Pedro Alves
On 2017-11-24 07:54, Pedro Alves wrote:
> On 11/24/2017 03:17 AM, Simon Marchi wrote:
>> On 2017-11-23 05:40 PM, Pedro Alves wrote:
>>> On 11/23/2017 09:13 PM, Simon Marchi wrote:
>>>
>>>> I have a patch in some branch that does essentially the same thing,
>>>> so
>>>> I was able to compare our approaches. In my version, I removed the
>>>> big allocation that is shared among pieces, and made each piece have
>>>> its own std::string. Unless we want to keep the current allocation
>>>> scheme for performance/memory usage reasons, I think that using
>>>> std::strings simplifies things in the parse_format_string function.
>>>> The format_pieces structure is replaced with an std::vector of
>>>> format_piece.
>>>
>>> Sounds like a step backwards to me. If it simplifies things, then
>>> it sounds like it might be because we're missing some utility,
>>> like string_view or something like that.
>>
>> I am not sure I understand, can you expand a little bit about what you
>> have in mind?
>
> I call it a step backwards because simplification is done at the
> cost of efficiency (individual heap allocations, changing the
> ownership model), when the original code avoided that. It's well known
> that
> standard C++ is missing basic string manipulation utilities, but that's
> no reason to force ourselves to consider std::string the ultimate tool,
> IMO.
> There's C++ the library, and then there's the C++ the language. We
> can't
> do anything about the latter, but we can extend the former. Later
> revisions
> of the standard are adding more facilities, like std::string_view in
> C++17, and
> now things like std::array_view / std::span (from gsl::span) have a
> very
> good chance of making their way into C++20. Many C++ projects end up
> reinventing something like those types, hence the attempts at
> standardization, making those types lingua franca for non-owning use
> cases.
Thanks for the detailed reply!
In my original reply, I said "Unless we want to keep the current
allocation scheme for performance/memory usage reasons", removing the
common allocation was done consciously. My thought was that the single
allocation scheme had not been chosen for performance reason, but for
simplicity. Computing how much to malloc for every piece would have
been very annoying. Making each piece own its string made the code
easier to reason about, but if the performance hit is not acceptable, I
understand.
>> What I meant is that is changes things like this:
>>
>> strncpy (current_substring, percent_loc, length_before_ll);
>> strcpy (current_substring + length_before_ll, "I64");
>> current_substring[length_before_ll + 3] =
>> percent_loc[length_before_ll + lcount];
>> current_substring += length_before_ll + 4;
>>
>> into this
>>
>> piece_string.assign (percent_loc, length_before_ll);
>> piece_string += "I64";
>> piece_string += percent_loc[length_before_ll + lcount];
>>
>> Less magical number, less playing with offsets, etc.
>
> I meant that you don't need for each piece_string to own its
> own deep copy of the string to get rid of the magical numbers.
> You can get that even with a couple C-like free-functions, like:
>
> // memcpy, null-terminate, and return advanced pointer.
> char *append (char *p, const char *str);
> char *append (char *p, const char *str, size_t len);
>
> and then:
>
> char *p = current_substring;
>
> p = append (p, percent_loc, length_before_ll);
> p = append (p, "I64");
> p = append (p, percent_loc[length_before_ll + lcount]);
>
> current_substring = p;
>
> You could wrap that in a class, like:
>
> /* Helper to build an null-terminated C string. */
> struct zstring_builder
> {
> /* POS is a pointer to the position in buffer to work on. */
> explicit zstring_builder (char *pos)
> : m_pos (pos)
> {}
>
> // memcpy, advance m_pos, null-terminate.
> zstring_builder &append (const char *s, size_type count);
> zstring_builder &append (const char* s);
>
> char *pos () { return m_pos; }
>
> private:
> char *m_pos;
> };
>
> Or:
>
> /* Helper to build a null-terminated C string on top
> of a vector. */
> struct zstring_builder
> {
> /* POS is a pointer to the position in buffer to work on. */
> explicit zstring_builder (std::vector<char> &buf, size_t pos);
>
> // memcpy, advance m_pos, null-terminate.
> zstring_builder &append (const char *s, size_type count);
> zstring_builder &append (const char* s);
>
> char *pos () { return &m_vector[m_pos]; }
>
> private:
> std::vector &m_buf;
> size_t m_pos;
> };
I agree that something like this is good if we want to keep the current
allocation scheme but make the code clearer. Since "append" continues
the same string piece, I think we would need some kind of "finish"
method (a bit like obstack) to be able to start a new string:
zstring_builder builder (buf);
builder.append ("Hello ");
builder.append ("world!");
const char *english = builder.finish ();
builder.append ("Bonjour ");
builder.append ("monde!");
const char *french = builder.finish ();
giving something like this:
Hello world!\0Bonjour monde!\0
> or something else along those lines, if you now make piece_string
> a zstring_builder, you can get similar, and just as readable code:
>
> zstring_builder piece_string (current_substring);
>
> piece_string.append (percent_loc, length_before_ll);
> piece_string.append ("I64");
> piece_string.append (percent_loc[length_before_ll + lcount]);
>
> current_substring = piece_string.pos ();
>
> Alternatively, we could have something like a zstring_ref
> class [1], as a non-owning mutable view of a C string (that never
> reallocates), guaranteed to be null-terminated, and then the code
> you shown would be just exactly the same. Open choices could be
> whether
> to save the string size inside zstring_ref (like string_view), or
> not saving a size, making it a really-thin wrapper around a
> pointer instead.
>
> std::string is great as an _owning_ SSO-enabled (which is sometimes a
> curse when embedded in structures...) container of characters. But
> when we don't want an owning container, the right tool / refactor can
> still make the code just as readable.
>
> [1] - I've thought several times because that something like a
> zstring_view
> (like string_view, but guarantees null-termination) would be handy in
> a lot of places. string_view isn't mutable, though, hence the
> alternative name instead. could be mut_string_view too, for example.
It's all performance/complexity tradeoffs, as usual :)
Simon
>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
Pedro> I call it a step backwards because simplification is done at the
Pedro> cost of efficiency (individual heap allocations, changing the
Pedro> ownership model), when the original code avoided that.
I agree with this principle in general, but I think that in this
particular case, the efficiency and/or allocation argument is not very
strong -- I just doubt this code is space- or time-sensitive.
So on the whole I think in this case it would be fine to use Simon's
patch.
Pedro> [1] - I've thought several times because that something like a zstring_view
Pedro> (like string_view, but guarantees null-termination) would be handy in
Pedro> a lot of places. string_view isn't mutable, though, hence the
Pedro> alternative name instead. could be mut_string_view too, for example.
In gdb I've often wanted something like "std::string API but
unique_xmalloc_ptr allocation policy". That way one could get the handy
string methods but allow nicer interfacing to things already using
xmalloc (I guess primarily libiberty but also readline and sometimes
BFD).
The main danger I see of views is that then one must reason about
lifetimes in order to use them. But in gdb maybe this can be handled
via some relatively simple rules, like "never store a view on the heap".
Tom
@@ -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 ();
}
@@ -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 */
@@ -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;
}
@@ -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;
- }
-}
-
@@ -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 */
@@ -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);
}
@@ -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. */