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 15/23] Use std::vector to avoid cleanups


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


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