This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFA] Fix leak in forward-search
- From: Pedro Alves <palves at redhat dot com>
- To: Philippe Waroquiers <philippe dot waroquiers at skynet dot be>, gdb-patches at sourceware dot org
- Date: Thu, 29 Nov 2018 15:42:02 +0000
- Subject: Re: [RFA] Fix leak in forward-search
- References: <20181127233328.5164-1-philippe.waroquiers@skynet.be>
On 11/27/2018 11:33 PM, Philippe Waroquiers wrote:
> Valgrind reports the below leak.
> Fix the leak by using xrealloc, even for the first allocation,
> as buf is static.
>
> ==29158== 5,888 bytes in 23 blocks are definitely lost in loss record 3,028 of 3,149
> ==29158== at 0x4C2BE2D: malloc (vg_replace_malloc.c:299)
> ==29158== by 0x41B557: xmalloc (common-utils.c:44)
> ==29158== by 0x60B7D9: forward_search_command(char const*, int) (source.c:1563)
> ==29158== by 0x40BA68: cmd_func(cmd_list_element*, char const*, int) (cli-decode.c:1888)
> ==29158== by 0x665300: execute_command(char const*, int) (top.c:630)
> ...
>
> gdb/ChangeLog
> 2018-11-28 Philippe Waroquiers <philippe.waroquiers@skynet.be>
>
> * source.c (forward_search_command): Fix leak by using
> xrealloc even for the first allocation in the loop, as buf
> is static.
At first sight it would seem like 'buf' was made static to avoid
allocating a growing buffer for each command invocation.
But then, if that were the case, then you'd want 'cursize' to be
static as well.
The patch is OK, but I think that replacing 'buf' and all that
manual buffer growing with a non-static gdb::def_vector<char> defined
outside the outer loop would be even better.
Thanks,
Pedro Alves