This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFA v2] arm-pikeos: software single step
On 2018-09-11 09:56, Joel Brobecker wrote:
> + symbol_table = (asymbol **) xmalloc (storage_needed);
> + make_cleanup (xfree, symbol_table);
It would make Tom really happy if you could avoid introducing a
cleanup :).
I would suggest using an std::vector<asymbol *>. If you'd rather not
do it
it's not a big deal, we can change it after.
Attached is the patch I am testing. I am wondering if this is the best
way, though. What do you think?
It looks good to me. You could use a range-for, because it's cool:
for (asymbol *sym : symbol_table)
but what you have is also fine.
One question I asked myself is whether we needed the std::vector at
all, as the building of the vector is a bit clunky in this situation.
As I understand it, this is mostly to automate the destruction of
the array. I was wondering whether we could do without the std::vector
entirely, and just handle the array without a cleanup, since the code
is simple enough that we can make sure it doesn't throw (I was hoping
that eg marking the function noexcept would help guaranty that). But
at the end of the day, although it's manageable in this case, I felt
it was better to go with the safer approach.
Well, it would be fine to not use a vector, but in any case I would
recommend the use of an object that ensures the memory is de-allocated
in any case, whether it is an std::vector, an std::unique_ptr or a
gdb::unique_xmalloc_ptr. I don't see any advantage of doing a manual
free over using one of those.
An alternative would be to use a VLA:
asymbol *symbol_table[max_number_of_symbols];
which I think ends up being like an alloca. But in this case there
could be a huge number of symbols I suppose, so I would avoid that.
Same remark with resizing the array: In practice, we don't need to do
it since we know the bounds and iterate over the elements without
accessing the them from the vector; but it's clearer and safer this
way.
Right, with the range-for you would need the resize though.
Simon