diff mbox

Memory leak in SVDR LSTE

Message ID dac12l$pj7$1@video.local.muempf.de
State New
Headers

Commit Message

Stefan Huelswitt July 4, 2005, 7:01 p.m. UTC 
  Hi,
I found that there is a memory leak in the SVDR LSTE command.
The socket file descriptor is fdopen'ed to dump the schedule
data, but the stream is never fclose'ed. The memory associated
with the stream is lost.

As simply fclosing the stream would close the socket as well, the
solution is to dup the socket first.

Patch attached.

Regards.

Comments

Klaus Schmidinger July 4, 2005, 8:04 p.m. UTC | #1 
Stefan Huelswitt wrote:
> Hi,
> I found that there is a memory leak in the SVDR LSTE command.
> The socket file descriptor is fdopen'ed to dump the schedule
> data, but the stream is never fclose'ed. The memory associated
> with the stream is lost.
> 
> As simply fclosing the stream would close the socket as well, the
> solution is to dup the socket first.
> 
> Patch attached.

Please make future patches so that they don't cause so many unnecessary
whitespace changes ;-)

Klaus
Stefan Huelswitt July 4, 2005, 8:16 p.m. UTC | #2 
On 04 Jul 2005 Klaus Schmidinger <Klaus.Schmidinger@cadsoft.de> wrote:
> Stefan Huelswitt wrote:
>
>> Patch attached.
> 
> Please make future patches so that they don't cause so many unnecessary
> whitespace changes ;-)

Well, one may argue about what is necessary and what not.

The patch adds one brace level, the indention has to be
adjusted. So I thing the whitespaces are necessary ;)

Or would you prefer patches with -b?

Regards.
Klaus Schmidinger July 4, 2005, 8:22 p.m. UTC | #3 
Stefan Huelswitt wrote:
> On 04 Jul 2005 Klaus Schmidinger <Klaus.Schmidinger@cadsoft.de> wrote:
> 
>>Stefan Huelswitt wrote:
>>
>>
>>>Patch attached.
>>
>>Please make future patches so that they don't cause so many unnecessary
>>whitespace changes ;-)
> 
> 
> Well, one may argue about what is necessary and what not.
> 
> The patch adds one brace level, the indention has to be
> adjusted. So I thing the whitespaces are necessary ;)

Oh, you're right. I just looked over it quickly and saw

+     if (fd) {
+       FILE *f = fdopen(fd, "w");

where the second line is not indented correctly, so that must
have triggered my message...

Klaus
diff mbox

Patch

--- vdr-1.3.24-orig/svdrp.c	2005-05-06 15:47:39.000000000 +0200
+++ vdr-current-1.3.24/svdrp.c	2005-07-04 20:50:52.000000000 +0200
@@ -776,18 +776,25 @@ 
               p = strtok_r(NULL, delim, &strtok_next);
               }
         }
-     FILE *f = fdopen(file, "w");
-     if (f) {
-        if (Schedule)
-           Schedule->Dump(f, "215-", DumpMode, AtTime);
-        else
-           Schedules->Dump(f, "215-", DumpMode, AtTime);
-        fflush(f);
-        Reply(215, "End of EPG data");
-        // don't 'fclose(f)' here!
-        }
+     int fd=dup(file);
+     if (fd) {
+       FILE *f = fdopen(fd, "w");
+       if (f) {
+          if (Schedule)
+             Schedule->Dump(f, "215-", DumpMode, AtTime);
+          else
+             Schedules->Dump(f, "215-", DumpMode, AtTime);
+          fflush(f);
+          Reply(215, "End of EPG data");
+          fclose(f);
+          }
+       else {
+          Reply(451, "Can't open file connection");
+          close(fd);
+          }
+       }
      else
-        Reply(451, "Can't open file connection");
+       Reply(451, "Can't dup stream descriptor");
      }
   else
      Reply(451, "Can't get EPG data");