Re: [RFA 15/23] Use std::vector to avoid cleanups

Pedro Alves
2017-06-02T19:22:46Z

On 05/03/2017 11:46 PM, Tom Tromey wrote:
> This patch introduces std::vector in several spots in gdb that were
> using xmalloc and a cleanup.
>
> The simple_search_memory change warrants a bit of extra attention. It
> was previously using malloc to try to handle memory allocation
> failures. I don't know whether this was actually important.

(
It still pains me a little to see std::vector used when we're going to
fill in the data anyway, because std::vector value/zero-initializes,
which is pointless in these cases. :-) I think I'll need to accept
that people dislike the idea of unique_ptr<gdb_byte[]>, so I'll stop
mentioning that, though at some point I think I may propose an
allocator that default-initializes, and then I'll nag people
to use _that_, like:

typedef std::vector<gdb_byte, default_init_allocator> gdb_byte_vec;

gdb_byte_vec buf (size);
)

> @@ -601,24 +599,18 @@ elf_rel_plt_read (minimal_symbol_reader &reader,
> OBJFILE the symbol is undefined and the objfile having NAME defined
> may not yet have been loaded. */
>
> - if (string_buffer_size < name_len + got_suffix_len + 1)
> - {
> - string_buffer_size = 2 * (name_len + got_suffix_len);
> - string_buffer = (char *) xrealloc (string_buffer, string_buffer_size);
> - }
> - memcpy (string_buffer, name, name_len);
> - memcpy (&string_buffer[name_len], SYMBOL_GOT_PLT_SUFFIX,
> - got_suffix_len + 1);
> + string_buffer.clear ();
> + string_buffer.insert (string_buffer.end (), name, name + name_len);
> + string_buffer.insert (string_buffer.end (), got_suffix,
> + got_suffix + got_suffix_len + 1);

So the patch all looks good to me, but it seems to me that in this
particular case, making this buffer be a std::string would clarify
the code:

string_buffer.assign (name, name_len);
string_buffer.append (got_suffix, got_suffix_len);

>
> - msym = record_minimal_symbol (reader, string_buffer,
> + msym = record_minimal_symbol (reader, string_buffer.data (),
> name_len + got_suffix_len,

and here maybe use string_buffer.size ()

(std::string is guaranteed to be null terminated.)

> true, address, mst_slot_got_plt, got_plt,
> objfile);

Thanks,
Pedro Alves