2015-07-28 10:42:10

by Anchit Narang

Subject: [PATCH] tools/l2test:Fixed memory leak

Freed memory allocated to filename before exiting.
Closed file before returning from do_send function.
---
tools/l2test.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/tools/l2test.c b/tools/l2test.c
index 1d458c4..a2e6d5d 100644
--- a/tools/l2test.c
+++ b/tools/l2test.c
@@ -961,6 +961,7 @@ static void do_send(int sk)
if (fd < 0) {
syslog(LOG_ERR, "Open failed: %s (%d)",
strerror(errno), errno);
+ free(filename);
exit(1);
}

@@ -974,6 +975,8 @@ static void do_send(int sk)
sent += len;
size -= len;
}
+
+ close(fd);
return;
} else {
for (i = 6; i < data_size; i++)
@@ -999,6 +1002,7 @@ static void do_send(int sk)
if (len < 0 || len != buflen) {
syslog(LOG_ERR, "Send failed: %s (%d)",
strerror(errno), errno);
+ free(filename);
exit(1);
}

--
1.7.9.5

2015-07-29 07:13:10

by Johan Hedberg

Subject: Re: [PATCH] tools/l2test:Fixed memory leak

Hi Anchit,

On Tue, Jul 28, 2015, Anchit Narang wrote:
> Freed memory allocated to filename before exiting.
> Closed file before returning from do_send function.
> ---
> tools/l2test.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/tools/l2test.c b/tools/l2test.c
> index 1d458c4..a2e6d5d 100644
> --- a/tools/l2test.c
> +++ b/tools/l2test.c
> @@ -961,6 +961,7 @@ static void do_send(int sk)
> if (fd < 0) {
> syslog(LOG_ERR, "Open failed: %s (%d)",
> strerror(errno), errno);
> + free(filename);
> exit(1);
> }

It's a bit awkward to go freeing up global variables here that were
allocated in the main() function. A cleaner way could be to use an
atexit() handler, but in this case I wonder if the extra allocation is
necessary at all, can't we just make 'filename' point at the right argv
element since that should be valid throughout the entire lifetime of the
process, i.e. instead of filename = strdup(optarg) it should be possible
to do filename = optarg (and change filename to const char *).

Johan