This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFA] C++-ify parse_format_string
- From: Simon Marchi <simon dot marchi at ericsson dot com>
- To: Tom Tromey <tom at tromey dot com>, <gdb-patches at sourceware dot org>
- Date: Thu, 23 Nov 2017 16:13:14 -0500
- Subject: Re: [RFA] C++-ify parse_format_string
- Authentication-results: sourceware.org; auth=none
- Authentication-results: spf=none (sender IP is ) smtp.mailfrom=simon dot marchi at ericsson dot com;
- References: <20171123164631.11055-1-tom@tromey.com>
- Spamdiagnosticmetadata: NSPM
- Spamdiagnosticoutput: 1:99
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