diff mbox

[RFA] Fix leak in event-top.c history expansion

Message ID 20190120182123.2223-1-philippe.waroquiers@skynet.be
State New
Headers show

Commit Message

Philippe Waroquiers Jan. 20, 2019, 6:21 p.m. UTC 
E.g. in gdb.base/default.exp, valgrind detects leaks such as
==17663== 1,438 bytes in 101 blocks are definitely lost in loss record 2,804 of 2,884
==17663==    at 0x4C2BE6D: malloc (vg_replace_malloc.c:309)
==17663==    by 0x418A17: xmalloc (common-utils.c:44)
==17663==    by 0x4E6F19C: history_expand (histexpand.c:1061)
==17663==    by 0x4B4490: handle_line_of_input(buffer*, char const*, int, char const*) (event-top.c:685)
==17663==    by 0x4B4562: command_line_handler(std::unique_ptr<char, gdb::xfree_deleter<char> >&&) (event-top.c:753)
...

Fix the leak by xfree-ing history_value if no expansion took place,
as a value is dynamically allocated even if no expansion took place.

Tested on debian/amd64, natively and under valgrind.
This was tested using the debian system readline library, version 7.0.3
(with upstream patches 002 and 003).

gdb/ChangeLog
2019-01-20  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

	* event-top.c (handle_line_of_input): xfree history_value if
	no expansion occurred.
---
 gdb/event-top.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Tom Tromey Jan. 20, 2019, 9:25 p.m. UTC | #1 
>>>>> "Philippe" == Philippe Waroquiers <philippe.waroquiers@skynet.be> writes:

Philippe> 2019-01-20  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
Philippe> 	* event-top.c (handle_line_of_input): xfree history_value if
Philippe> 	no expansion occurred.

How about turning history_value into a unique_xmalloc_ptr,
then use .release in the final assignment from it?
That would eliminate another xfree and IMO be a bit more future-proof.

Tom
diff mbox

Patch

diff --git a/gdb/event-top.c b/gdb/event-top.c
index 3d3d6275d7..93ea13e3ed 100644
--- a/gdb/event-top.c
+++ b/gdb/event-top.c
@@ -705,6 +705,8 @@  handle_line_of_input (struct buffer *cmd_line_buffer,
 	  cmd_line_buffer->buffer_size = len + 1;
 	  cmd = history_value;
 	}
+      else
+	xfree (history_value);
     }
 
   /* If we just got an empty line, and that is supposed to repeat the