This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFA] C++-ify parse_format_string


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]