Patchwork [libfortran] PR47296 Segfault when running out of file descriptors

login
register
mail settings
Submitter Janne Blomqvist
Date Jan. 16, 2011, 8:11 p.m.
Message ID <AANLkTi=R6qTThJs6eo05PqNR+cowFwyU=ON90QWEY3Hu@mail.gmail.com>
Download mbox | patch
Permalink /patch/79098/
State New
Headers show

Comments

Janne Blomqvist - Jan. 16, 2011, 8:11 p.m.
Hello,

the attached patch fixes the PR, the reason for the segfault was that
even if opening a temporary file fails we still access the file name
info. Thus don't free it in the unix.c:tempfile() function; freeing
opp->file is taken care of by the cleanup in open.c:new_unit() which
is the only caller. Thus this does not cause a memory leak (checked
with valgrind).

Regtested on x86_64-unknown-linux-gnu, Ok for trunk?
Jerry DeLisle - Jan. 17, 2011, 2:37 a.m.
On 01/16/2011 12:11 PM, Janne Blomqvist wrote:
> Hello,
>
> the attached patch fixes the PR, the reason for the segfault was that
> even if opening a temporary file fails we still access the file name
> info. Thus don't free it in the unix.c:tempfile() function; freeing
> opp->file is taken care of by the cleanup in open.c:new_unit() which
> is the only caller. Thus this does not cause a memory leak (checked
> with valgrind).
>
> Regtested on x86_64-unknown-linux-gnu, Ok for trunk?
>

yes, this is fine.  Go ahead and commit.  I am still doing some testing on 
cygwin, but my initial tests are OK.  The only thing that has been holding me up 
is some build issues plus the time it takes to even try a build.

Thanks,

Jerry
Janne Blomqvist - Jan. 17, 2011, 5:50 a.m.
On Mon, Jan 17, 2011 at 04:37, Jerry DeLisle <jvdelisle@frontier.com> wrote:
> On 01/16/2011 12:11 PM, Janne Blomqvist wrote:
>>
>> Hello,
>>
>> the attached patch fixes the PR, the reason for the segfault was that
>> even if opening a temporary file fails we still access the file name
>> info. Thus don't free it in the unix.c:tempfile() function; freeing
>> opp->file is taken care of by the cleanup in open.c:new_unit() which
>> is the only caller. Thus this does not cause a memory leak (checked
>> with valgrind).
>>
>> Regtested on x86_64-unknown-linux-gnu, Ok for trunk?
>>
>
> yes, this is fine.  Go ahead and commit.  I am still doing some testing on
> cygwin, but my initial tests are OK.  The only thing that has been holding
> me up is some build issues plus the time it takes to even try a build.

Sending        libgfortran/ChangeLog
Sending        libgfortran/io/unix.c
Transmitting file data ..
Committed revision 168888.

Thanks.

Patch

diff --git a/libgfortran/io/unix.c b/libgfortran/io/unix.c
index e66560f..f34ee63 100644
--- a/libgfortran/io/unix.c
+++ b/libgfortran/io/unix.c
@@ -1084,13 +1084,8 @@  tempfile (st_parameter_open *opp)
   while (fd == -1 && errno == EEXIST);
 #endif /* HAVE_MKSTEMP */
 
-  if (fd < 0)
-    free (template);
-  else
-    {
-      opp->file = template;
-      opp->file_len = strlen (template);	/* Don't include trailing nul */
-    }
+  opp->file = template;
+  opp->file_len = strlen (template);	/* Don't include trailing nul */
 
   return fd;
 }