[1/2] Simplify write_inferior_memory

Message ID 20190814164814.29367-2-tromey@adacore.com
State New
Headers show
Series
  • Small memory-writing gdbserver improvements
Related show

Commit Message

Tom Tromey Aug. 14, 2019, 4:48 p.m.
gdbserver's write_inferior_memory uses a static variable to avoid
memory leaks, and has a comment referring to the lack of cleanups.
This patch removes this comment and the code in favor of a
straightforward use of std::vector.

gdb/gdbserver/ChangeLog
2019-08-14  Tom Tromey  <tromey@adacore.com>

	* target.c (write_inferior_memory): Use std::vector.
---
 gdb/gdbserver/ChangeLog |  4 ++++
 gdb/gdbserver/target.c  | 22 +++++-----------------
 2 files changed, 9 insertions(+), 17 deletions(-)

-- 
2.20.1

Comments

Kevin Buettner Aug. 15, 2019, 1:59 a.m. | #1
On Wed, 14 Aug 2019 10:48:13 -0600
Tom Tromey <tromey@adacore.com> wrote:

> gdbserver's write_inferior_memory uses a static variable to avoid

> memory leaks, and has a comment referring to the lack of cleanups.

> This patch removes this comment and the code in favor of a

> straightforward use of std::vector.

> 

> gdb/gdbserver/ChangeLog

> 2019-08-14  Tom Tromey  <tromey@adacore.com>

> 

> 	* target.c (write_inferior_memory): Use std::vector.


LGTM.

Kevin
Pedro Alves Aug. 15, 2019, 3:51 p.m. | #2
On 8/14/19 5:48 PM, Tom Tromey wrote:
> +  std::vector<unsigned char> buffer (myaddr, myaddr + len);


gdb::byte_vector 

> +  check_mem_write (memaddr, buffer.data (), myaddr, len);

> +  return (*the_target->write_memory) (memaddr, buffer.data (), len);


Thanks,
Pedro Alves
Tom Tromey Aug. 15, 2019, 5:22 p.m. | #3
>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:


Pedro> On 8/14/19 5:48 PM, Tom Tromey wrote:
>> +  std::vector<unsigned char> buffer (myaddr, myaddr + len);


Pedro> gdb::byte_vector 

Thanks, I forgot about that.  I'll fix it momentarily.

Tom

Patch

diff --git a/gdb/gdbserver/target.c b/gdb/gdbserver/target.c
index 25f91eeba73..2587d8aa550 100644
--- a/gdb/gdbserver/target.c
+++ b/gdb/gdbserver/target.c
@@ -150,23 +150,11 @@  int
 write_inferior_memory (CORE_ADDR memaddr, const unsigned char *myaddr,
 		       int len)
 {
-  /* Lacking cleanups, there is some potential for a memory leak if the
-     write fails and we go through error().  Make sure that no more than
-     one buffer is ever pending by making BUFFER static.  */
-  static unsigned char *buffer = 0;
-  int res;
-
-  if (buffer != NULL)
-    free (buffer);
-
-  buffer = (unsigned char *) xmalloc (len);
-  memcpy (buffer, myaddr, len);
-  check_mem_write (memaddr, buffer, myaddr, len);
-  res = (*the_target->write_memory) (memaddr, buffer, len);
-  free (buffer);
-  buffer = NULL;
-
-  return res;
+  /* Make a copy of the data because check_mem_write may need to
+     update it.  */
+  std::vector<unsigned char> buffer (myaddr, myaddr + len);
+  check_mem_write (memaddr, buffer.data (), myaddr, len);
+  return (*the_target->write_memory) (memaddr, buffer.data (), len);
 }
 
 /* See target/target.h.  */