Re: [RFA v2] arm-pikeos: software single step

[Simon Marchi <simon.marchi@polymtl.ca>]

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