This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFA 15/23] Use std::vector to avoid cleanups
- From: Pedro Alves <palves at redhat dot com>
- To: Tom Tromey <tom at tromey dot com>, gdb-patches at sourceware dot org
- Date: Fri, 2 Jun 2017 20:22:45 +0100
- Subject: Re: [RFA 15/23] Use std::vector to avoid cleanups
- Authentication-results: sourceware.org; auth=none
- Authentication-results: ext-mx05.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com
- Authentication-results: ext-mx05.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=palves at redhat dot com
- Dkim-filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 61B77334584
- Dmarc-filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 61B77334584
- References: <20170503224626.2818-1-tom@tromey.com> <20170503224626.2818-16-tom@tromey.com>
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