[RFA] Fix two memory leaks

[RFA] Fix two memory leaks

[Keith Seitz]

[-- Attachment #1: Type: text/plain, Size: 410 bytes --]

Hi,

While testing some recent work, I noticed that valgrind was reporting 
two memory leaks related to linespec parsing/SaL resolution.

The following patch fixes both leaks.

Keith

ChangeLog
2012-10-23  Keith Seitz  <keiths@redhat.com>

	* breakpoint.c (clear_command): Add cleanup for
	sals.sals if an argument is given.

	* linespec.c (parse_linespec): Do cleanups after
	parsing a convenience variable.


[-- Attachment #2: mem-leaks.patch --]
[-- Type: text/x-patch, Size: 1281 bytes --]

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 8eeeacf..8b6b635 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -11788,6 +11788,7 @@ clear_command (char *arg, int from_tty)
       sals = decode_line_with_current_source (arg,
 					      (DECODE_LINE_FUNFIRSTLINE
 					       | DECODE_LINE_LIST_MODE));
+      make_cleanup (xfree, sals.sals);
       default_match = 0;
     }
   else
diff --git a/gdb/linespec.c b/gdb/linespec.c
index 06634d2..e9918b1 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -2095,20 +2095,17 @@ parse_linespec (linespec_parser *parser, char **argptr)
       cleanup = make_cleanup (xfree, var);
       PARSER_RESULT (parser)->line_offset
 	= linespec_parse_variable (PARSER_STATE (parser), var);
+      do_cleanups (cleanup);
 
       /* If a line_offset wasn't found (VAR is the name of a user
 	 variable/function), then skip to normal symbol processing.  */
       if (PARSER_RESULT (parser)->line_offset.sign != LINE_OFFSET_UNKNOWN)
 	{
-	  discard_cleanups (cleanup);
-
 	  /* Consume this token.  */
 	  linespec_lexer_consume_token (parser);
 
 	  goto convert_to_sals;
 	}
-
-      do_cleanups (cleanup);
     }
   else if (token.type != LSTOKEN_STRING && token.type != LSTOKEN_NUMBER)
     unexpected_linespec_error (parser);

Re: [RFA] Fix two memory leaks

[Joel Brobecker]

Hi Keith,

Tsk, tsk, tsk - can we have two separate patches, please? :-)
Seriously, it does make it easier if we have unexpected regressions...

> ChangeLog
> 2012-10-23  Keith Seitz  <keiths@redhat.com>
> 
> 	* breakpoint.c (clear_command): Add cleanup for
> 	sals.sals if an argument is given.

This one looks good to me. But I'm wonderin if we shouldn't also
document the fact that decode_line_with_current_source returns
a structure that contains newly allocated memory?

> 	* linespec.c (parse_linespec): Do cleanups after
> 	parsing a convenience variable.

This one looks good to me too.

You might want to wait for a day or two before checking in, as these
files are not within my area of expertise. But I verified that the
memory is allocated on the heap, and leaked if not free'ed.

Cheers,
-- 
Joel

Re: [RFA] Fix two memory leaks

[Tom Tromey]

>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:

Joel> This one looks good to me. But I'm wonderin if we shouldn't also
Joel> document the fact that decode_line_with_current_source returns
Joel> a structure that contains newly allocated memory?

The docs for it should probably reference decode_line_full.

>> * linespec.c (parse_linespec): Do cleanups after
>> parsing a convenience variable.

Joel> This one looks good to me too.

Joel> You might want to wait for a day or two before checking in, as these
Joel> files are not within my area of expertise. But I verified that the
Joel> memory is allocated on the heap, and leaked if not free'ed.

I think they are both ok as well.

Tom