[patch]: Fix memory leak of c-exp.y

[patch]: Fix memory leak of c-exp.y

[teawater]

Sorry that the format of this patch is not very well. So I send it again.

c-exp.y has a memory leak in function parse_number. char *s is malloc
at line 1211.
There are returns at lines 1137, 1147, and 1157 without calling free.
This patch is for the GDB CVS version.

ChangeLog:
2008-06-21  Hui Zhu <teawater@gmail.com>
       * gdb/c-exp.y: Fix memory leak of function parse_number

--- a/gdb/c-exp.y
+++ b/gdb/c-exp.y
@@ -1134,6 +1134,7 @@ parse_number (p, len, parsed_float, puti
 	    = builtin_type (current_gdbarch)->builtin_decfloat;
 	  decimal_from_string (putithere->typed_val_decfloat.val, 4, p);
 	  p[len] = saved_char;
+	  free (s);
 	  return (DECFLOAT);
 	}

@@ -1144,6 +1145,7 @@ parse_number (p, len, parsed_float, puti
 	    = builtin_type (current_gdbarch)->builtin_decdouble;
 	  decimal_from_string (putithere->typed_val_decfloat.val, 8, p);
 	  p[len] = saved_char;
+	  free (s);
 	  return (DECFLOAT);
 	}

@@ -1154,6 +1156,7 @@ parse_number (p, len, parsed_float, puti
 	    = builtin_type (current_gdbarch)->builtin_declong;
 	  decimal_from_string (putithere->typed_val_decfloat.val, 16, p);
 	  p[len] = saved_char;
+	  free (s);
 	  return (DECFLOAT);
 	}

Re: [patch]: Fix memory leak of c-exp.y

[Jan Kratochvil]

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

On Tue, 24 Jun 2008 08:32:44 +0200, teawater wrote:
> ChangeLog:
> 2008-06-21  Hui Zhu <teawater@gmail.com>
>        * gdb/c-exp.y: Fix memory leak of function parse_number

Agreed with the fix just IMO the block of code needs more cleanups.  [attached]
The leak was brought in by:
  http://sourceware.org/ml/gdb-patches/2007-10/msg00395.html

Just please check the GNU Coding Standards document for the ChangeLog style, it
should have been more like:

2008-06-21  Hui Zhu  <teawater@gmail.com>

	* c-exp.y (parse_number): Fix a memory leak.


Thanks,
Jan

[-- Attachment #2: gdb-c-exp-dfp-leaketc.patch --]
[-- Type: text/plain, Size: 2525 bytes --]

2008-06-24  Jan Kratochvil  <jan.kratochvil@redhat.com>

	Fix a memory leak found by Hui Zhu <teawater@gmail.com>.
	* c-exp.y (parse_number): Move the S and SAVED_CHAR initialization
	after the DECFLOAT detection to fix a memory leak.  Remove the
	redundant NUM initialization.  Protect the DECFLOAT detection memory
	access before the P block.  Restore the P memory content for the
	DECFLOAT detection.

--- ./gdb/c-exp.y	9 Jun 2008 19:25:14 -0000	1.45
+++ ./gdb/c-exp.y	24 Jun 2008 13:03:26 -0000
@@ -1118,45 +1118,46 @@ parse_number (p, len, parsed_float, puti
   if (parsed_float)
     {
       /* It's a float since it contains a point or an exponent.  */
-      char *s = malloc (len);
-      int num = 0;	/* number of tokens scanned by scanf */
-      char saved_char = p[len];
-
-      p[len] = 0;	/* null-terminate the token */
+      char *s;
+      int num;	/* number of tokens scanned by scanf */
+      char saved_char;
 
       /* If it ends at "df", "dd" or "dl", take it as type of decimal floating
          point.  Return DECFLOAT.  */
 
-      if (p[len - 2] == 'd' && p[len - 1] == 'f')
+      if (len >= 2 && p[len - 2] == 'd' && p[len - 1] == 'f')
 	{
 	  p[len - 2] = '\0';
 	  putithere->typed_val_decfloat.type
 	    = builtin_type (current_gdbarch)->builtin_decfloat;
 	  decimal_from_string (putithere->typed_val_decfloat.val, 4, p);
-	  p[len] = saved_char;
-	  return (DECFLOAT);
+	  p[len - 2] = 'd';
+	  return DECFLOAT;
 	}
 
-      if (p[len - 2] == 'd' && p[len - 1] == 'd')
+      if (len >= 2 && p[len - 2] == 'd' && p[len - 1] == 'd')
 	{
 	  p[len - 2] = '\0';
 	  putithere->typed_val_decfloat.type
 	    = builtin_type (current_gdbarch)->builtin_decdouble;
 	  decimal_from_string (putithere->typed_val_decfloat.val, 8, p);
-	  p[len] = saved_char;
-	  return (DECFLOAT);
+	  p[len - 2] = 'd';
+	  return DECFLOAT;
 	}
 
-      if (p[len - 2] == 'd' && p[len - 1] == 'l')
+      if (len >= 2 && p[len - 2] == 'd' && p[len - 1] == 'l')
 	{
 	  p[len - 2] = '\0';
 	  putithere->typed_val_decfloat.type
 	    = builtin_type (current_gdbarch)->builtin_declong;
 	  decimal_from_string (putithere->typed_val_decfloat.val, 16, p);
-	  p[len] = saved_char;
-	  return (DECFLOAT);
+	  p[len - 2] = 'd';
+	  return DECFLOAT;
 	}
 
+      s = malloc (len);
+      saved_char = p[len];
+      p[len] = 0;	/* null-terminate the token */
       num = sscanf (p, "%" DOUBLEST_SCAN_FORMAT "%s",
 		    &putithere->typed_val_float.dval, s);
       p[len] = saved_char;	/* restore the input stream */

Re: [patch]: Fix memory leak of c-exp.y

[teawater]

On Tue, Jun 24, 2008 at 21:35, Jan Kratochvil <jan.kratochvil@redhat.com> wrote:
> On Tue, 24 Jun 2008 08:32:44 +0200, teawater wrote:
>> ChangeLog:
>> 2008-06-21  Hui Zhu <teawater@gmail.com>
>>        * gdb/c-exp.y: Fix memory leak of function parse_number
>
> Agreed with the fix just IMO the block of code needs more cleanups.  [attached]
> The leak was brought in by:
>  http://sourceware.org/ml/gdb-patches/2007-10/msg00395.html
>
> Just please check the GNU Coding Standards document for the ChangeLog style, it
> should have been more like:
>
> 2008-06-21  Hui Zhu  <teawater@gmail.com>
>
>        * c-exp.y (parse_number): Fix a memory leak.
>

OK. Thank you Jan.

Hui

Re: [patch]: Fix memory leak of c-exp.y

[Joel Brobecker]

Hi teawater,

> 2008-06-21  Hui Zhu <teawater@gmail.com>
>        * gdb/c-exp.y: Fix memory leak of function parse_number

Instead of adding calls to free, I looks like this string is actually
completely local, so how about replacing the call to malloc with a
call to alloca, and then get rid of all calls to free instead?

-- 
Joel

Re: [patch]: Fix memory leak of c-exp.y

[Daniel Jacobowitz]

On Wed, Jun 25, 2008 at 08:43:03AM -0400, Joel Brobecker wrote:
> Hi teawater,
> 
> > 2008-06-21  Hui Zhu <teawater@gmail.com>
> >        * gdb/c-exp.y: Fix memory leak of function parse_number
> 
> Instead of adding calls to free, I looks like this string is actually
> completely local, so how about replacing the call to malloc with a
> call to alloca, and then get rid of all calls to free instead?

Let's go with Jan's version for now - he's fixed a few related
problems in the same function.  Jan, that patch is OK.

-- 
Daniel Jacobowitz
CodeSourcery

[patch]: Fix memory leak of c-exp.y

[teawater]

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

c-exp.y has a memory leak in function parse_number. char *s is malloc
at line 1211.
There are returns at lines 1137, 1147, and 1157 without calling free.
This patch is for the GDB CVS version.

ChangeLog:
	* gdb/c-exp.y: Fix memory leak of function parse_number

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: c-exp_leak.patch --]
[-- Type: text/x-diff; name=c-exp_leak.patch, Size: 805 bytes --]

--- a/gdb/c-exp.y
+++ b/gdb/c-exp.y
@@ -1134,6 +1134,7 @@ parse_number (p, len, parsed_float, puti
 	    = builtin_type (current_gdbarch)->builtin_decfloat;
 	  decimal_from_string (putithere->typed_val_decfloat.val, 4, p);
 	  p[len] = saved_char;
+	  free (s);
 	  return (DECFLOAT);
 	}
 
@@ -1144,6 +1145,7 @@ parse_number (p, len, parsed_float, puti
 	    = builtin_type (current_gdbarch)->builtin_decdouble;
 	  decimal_from_string (putithere->typed_val_decfloat.val, 8, p);
 	  p[len] = saved_char;
+	  free (s);
 	  return (DECFLOAT);
 	}
 
@@ -1154,6 +1156,7 @@ parse_number (p, len, parsed_float, puti
 	    = builtin_type (current_gdbarch)->builtin_declong;
 	  decimal_from_string (putithere->typed_val_decfloat.val, 16, p);
 	  p[len] = saved_char;
+	  free (s);
 	  return (DECFLOAT);
 	}