diff mbox

[v2,12/19] read/write_pieced_value: Drop 'buffer_size' variable

Message ID 1494352015-10465-13-git-send-email-arnez@linux.vnet.ibm.com
State New, archived
Headers

Commit Message

Andreas Arnez May 9, 2017, 5:46 p.m. UTC 
  When the variable 'buffer_size' in read_pieced_value and
write_pieced_value was introduced, it was needed for tracking the buffer's
allocated size.  Now that the buffer's data type has been changed to a
std::vector, the variable is no longer necessary; so remove it.

gdb/ChangeLog:

	* dwarf2loc.c (read_pieced_value): Remove buffer_size variable.
	(write_pieced_value): Likewise.
---
 gdb/dwarf2loc.c | 14 ++------------
 1 file changed, 2 insertions(+), 12 deletions(-)

Comments

Yao Qi May 16, 2017, 2:08 p.m. UTC | #1 
Andreas Arnez <arnez@linux.vnet.ibm.com> writes:

Patch is good to me, a nit below,

> @@ -1806,12 +1805,8 @@ read_pieced_value (struct value *v)
>  	this_size_bits = max_offset - offset;
>  
>        this_size = (this_size_bits + source_offset_bits % 8 + 7) / 8;
> +      buffer.reserve (this_size);

I noticed that buffer is only used within the for loop, so probably we
can move it into the loop, and do "std::vector<gdb_byte> buffer (this_size);"

This change makes troubles for you to rebase your following patches, so
if you want to change it, you can do it in another patch, 20/19, for
example.

>        source_offset = source_offset_bits / 8;
> -      if (buffer_size < this_size)
> -	{
> -	  buffer_size = this_size;
> -	  buffer.reserve (buffer_size);
> -	}
>        intermediate_buffer = buffer.data ();
>  
>        /* Copy from the source to DEST_BUFFER.  */
Andreas Arnez May 16, 2017, 5:51 p.m. UTC | #2 
On Tue, May 16 2017, Yao Qi wrote:

> Andreas Arnez <arnez@linux.vnet.ibm.com> writes:
>
> Patch is good to me, a nit below,
>
>> @@ -1806,12 +1805,8 @@ read_pieced_value (struct value *v)
>>  	this_size_bits = max_offset - offset;
>>  
>>        this_size = (this_size_bits + source_offset_bits % 8 + 7) / 8;
>> +      buffer.reserve (this_size);
>
> I noticed that buffer is only used within the for loop, so probably we
> can move it into the loop, and do "std::vector<gdb_byte> buffer (this_size);"

I thought there might be a slight performance benefit in keeping the
buffer variable across loop iterations and just '.reserve' more space
when needed.  This was probably the original intention as well.

--
Andreas
diff mbox

Patch

diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
index da0e8c3..5c29cf6 100644
--- a/gdb/dwarf2loc.c
+++ b/gdb/dwarf2loc.c
@@ -1761,7 +1761,6 @@  read_pieced_value (struct value *v)
   gdb_byte *contents;
   struct piece_closure *c
     = (struct piece_closure *) value_computed_closure (v);
-  size_t buffer_size = 0;
   std::vector<gdb_byte> buffer;
   int bits_big_endian
     = gdbarch_bits_big_endian (get_type_arch (value_type (v)));
@@ -1806,12 +1805,8 @@  read_pieced_value (struct value *v)
 	this_size_bits = max_offset - offset;
 
       this_size = (this_size_bits + source_offset_bits % 8 + 7) / 8;
+      buffer.reserve (this_size);
       source_offset = source_offset_bits / 8;
-      if (buffer_size < this_size)
-	{
-	  buffer_size = this_size;
-	  buffer.reserve (buffer_size);
-	}
       intermediate_buffer = buffer.data ();
 
       /* Copy from the source to DEST_BUFFER.  */
@@ -1929,7 +1924,6 @@  write_pieced_value (struct value *to, struct value *from)
   const gdb_byte *contents;
   struct piece_closure *c
     = (struct piece_closure *) value_computed_closure (to);
-  size_t buffer_size = 0;
   std::vector<gdb_byte> buffer;
   int bits_big_endian
     = gdbarch_bits_big_endian (get_type_arch (value_type (to)));
@@ -1986,11 +1980,7 @@  write_pieced_value (struct value *to, struct value *from)
 	}
       else
 	{
-	  if (buffer_size < this_size)
-	    {
-	      buffer_size = this_size;
-	      buffer.reserve (buffer_size);
-	    }
+	  buffer.reserve (this_size);
 	  source_buffer = buffer.data ();
 	  need_bitwise = 1;
 	}