diff mbox

[Fortran] PR55469 - fix I/O memory leaks in case of failure and iostat= being present

Message ID 50B7492B.5030702@net-b.de
State New
Headers show

Commit Message

Tobias Burnus Nov. 29, 2012, 11:38 a.m. UTC 
Tobias Burnus wrote:
> l_push_char allocates memory which is freed with free_line. However, 
> currently, the memory is not always freed when calling generate_error. 
> If one aborts, that's fine. However, generate_error can also set the 
> iostat variable.

Updated version: Corrected PR number - and ensured that if convert_real 
fails, free_saved is called (cf. additional test case in the PR).

Build and regtested on x86-64-gnu-linux.
OK for the trunk?

Tobias

Comments

Janne Blomqvist Nov. 29, 2012, 12:02 p.m. UTC | #1 
On Thu, Nov 29, 2012 at 1:38 PM, Tobias Burnus <burnus@net-b.de> wrote:
> Tobias Burnus wrote:
>>
>> l_push_char allocates memory which is freed with free_line. However,
>> currently, the memory is not always freed when calling generate_error. If
>> one aborts, that's fine. However, generate_error can also set the iostat
>> variable.
>
>
> Updated version: Corrected PR number - and ensured that if convert_real
> fails, free_saved is called (cf. additional test case in the PR).
>
> Build and regtested on x86-64-gnu-linux.
> OK for the trunk?

IIRC this is supposed to be a cache than can be subsequently reused
without freeing and allocating it again. So it might be better to free
it once when the unit is closed.

At some point this line buffer should be removed completely and
replaced with the fbuf_*() machinery. But so far nobody has found the
time to work on that.
Tobias Burnus Oct. 1, 2013, 8:53 p.m. UTC | #2 
I have committed the patch after Janne's approval on IRC  and fresh 
building/regtesting as Rev. 203086. See 
http://gcc.gnu.org/ml/fortran/2012-11/msg00092.html

As suggested by Janne, I will backport it to 4.8 in in a bunch of days.

Tobias

On November 29, 2012 12:38, Tobias Burnus wrote:
> Tobias Burnus wrote:
>> l_push_char allocates memory which is freed with free_line. However, 
>> currently, the memory is not always freed when calling 
>> generate_error. If one aborts, that's fine. However, generate_error 
>> can also set the iostat variable.
>
> Updated version: Corrected PR number - and ensured that if 
> convert_real fails, free_saved is called (cf. additional test case in 
> the PR).
>
> Build and regtested on x86-64-gnu-linux.
> OK for the trunk?
>
> Tobias
diff mbox

Patch

2012-11-29  Tobias Burnus  <burnus@net-b.de>

	PR fortran/55469
	* io/list_read (parse_repeat, read_integer, read_character,
	parse_real, read_real, check_type, list_formatted_read_scalar,
	finish_list_read): Call list_free.

diff --git a/libgfortran/io/list_read.c b/libgfortran/io/list_read.c
index 403e719..5063c36 100644
--- a/libgfortran/io/list_read.c
+++ b/libgfortran/io/list_read.c
@@ -617,6 +617,7 @@  parse_repeat (st_parameter_dt *dtp)
   free_saved (dtp);
   if (c == EOF)
     {
+      free_line (dtp);
       hit_eof (dtp);
       return 1;
     }
@@ -905,11 +906,14 @@  read_integer (st_parameter_dt *dtp, int length)
   free_saved (dtp);  
   if (c == EOF)
     {
+      free_line (dtp);
       hit_eof (dtp);
       return;
     }
   else if (c != '\n')
     eat_line (dtp);
+
+  free_line (dtp);
   snprintf (message, MSGLEN, "Bad integer for item %d in list input",
 	      dtp->u.p.item_count);
   generate_error (&dtp->common, LIBERROR_READ_VALUE, message);
@@ -1078,7 +1082,6 @@  read_character (st_parameter_dt *dtp, int length __attribute__ ((unused)))
       unget_char (dtp, c);
       eat_separator (dtp);
       dtp->u.p.saved_type = BT_CHARACTER;
-      free_line (dtp);
     }
   else 
     {
@@ -1087,10 +1090,12 @@  read_character (st_parameter_dt *dtp, int length __attribute__ ((unused)))
 		  dtp->u.p.item_count);
       generate_error (&dtp->common, LIBERROR_READ_VALUE, message);
     }
+  free_line (dtp);
   return;
 
  eof:
   free_saved (dtp);
+  free_line (dtp);
   hit_eof (dtp);
 }
 
@@ -1283,11 +1288,14 @@  parse_real (st_parameter_dt *dtp, void *buffer, int length)
   free_saved (dtp);
   if (c == EOF)
     {
+      free_line (dtp);
       hit_eof (dtp);
       return 1;
     }
   else if (c != '\n')
     eat_line (dtp);
+
+  free_line (dtp);
   snprintf (message, MSGLEN, "Bad floating point number for item %d",
 	      dtp->u.p.item_count);
   generate_error (&dtp->common, LIBERROR_READ_VALUE, message);
@@ -1387,11 +1395,14 @@  eol_4:
   free_saved (dtp);
   if (c == EOF)
     {
+      free_line (dtp);
       hit_eof (dtp);
       return;
     }
   else if (c != '\n')   
     eat_line (dtp);
+
+  free_line (dtp);
   snprintf (message, MSGLEN, "Bad complex value in item %d of list input",
 	      dtp->u.p.item_count);
   generate_error (&dtp->common, LIBERROR_READ_VALUE, message);
@@ -1624,7 +1635,10 @@  read_real (st_parameter_dt *dtp, void * dest, int length)
   eat_separator (dtp);
   push_char (dtp, '\0');
   if (convert_real (dtp, dest, dtp->u.p.saved_string, length))
-    return;
+    {
+      free_saved (dtp);
+      return;
+    }
 
   free_saved (dtp);
   dtp->u.p.saved_type = BT_REAL;
@@ -1762,12 +1776,14 @@  read_real (st_parameter_dt *dtp, void * dest, int length)
   free_saved (dtp);
   if (c == EOF)
     {
+      free_line (dtp);
       hit_eof (dtp);
       return;
     }
   else if (c != '\n')
     eat_line (dtp);
 
+  free_line (dtp);
   snprintf (message, MSGLEN, "Bad real number in item %d of list input",
 	      dtp->u.p.item_count);
   generate_error (&dtp->common, LIBERROR_READ_VALUE, message);
@@ -1784,6 +1800,7 @@  check_type (st_parameter_dt *dtp, bt type, int len)
 
   if (dtp->u.p.saved_type != BT_UNKNOWN && dtp->u.p.saved_type != type)
     {
+      free_line (dtp);
       snprintf (message, MSGLEN, "Read type %s where %s was expected for item %d",
 		  type_name (dtp->u.p.saved_type), type_name (type),
 		  dtp->u.p.item_count);
@@ -1797,6 +1814,7 @@  check_type (st_parameter_dt *dtp, bt type, int len)
 
   if (dtp->u.p.saved_length != len)
     {
+      free_line (dtp);
       snprintf (message, MSGLEN,
 		  "Read kind %d %s where kind %d is required for item %d",
 		  dtp->u.p.saved_length, type_name (dtp->u.p.saved_type), len,
@@ -1970,7 +1988,10 @@  list_formatted_read_scalar (st_parameter_dt *dtp, bt type, void *p,
 
 cleanup:
   if (err == LIBERROR_END)
-    hit_eof (dtp);
+    {
+      free_line (dtp);
+      hit_eof (dtp);
+    }
   return err;
 }
 
@@ -2018,7 +2039,10 @@  finish_list_read (st_parameter_dt *dtp)
 
   err = eat_line (dtp);
   if (err == LIBERROR_END)
-    hit_eof (dtp);
+    {
+      free_line (dtp);
+      hit_eof (dtp);
+    }
 }
 
 /*			NAMELIST INPUT