[ovs-dev,1/1] vlog: fix memory leak in vlog_set_log_file() function
diff mbox series

Message ID 1570016689-30777-1-git-send-email-damjan.skvarc@gmail.com
State New
Headers show
Series
  • [ovs-dev,1/1] vlog: fix memory leak in vlog_set_log_file() function
Related show

Commit Message

Damijan Skvarc Oct. 2, 2019, 11:44 a.m. UTC
memory leak happens in case previously closed log file was reopened again,
for example:

ovs-appctl vlog/close
ovs-appctl vlog/reopen

memory leak is reported by valgrind in a form:

==4463== 76 bytes in 1 blocks are definitely lost in loss record 322 of 344
==4463==    at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==4463==    by 0x534314: xmalloc (util.c:138)
==4463==    by 0x534384: xmemdup0 (util.c:168)
==4463==    by 0x53ACB9: vlog_set_log_file (vlog.c:403)
==4463==    by 0x53AEDC: vlog_reopen_log_file (vlog.c:434)
==4463==    by 0x53AF22: vlog_unixctl_reopen (vlog.c:683)
==4463==    by 0x533730: process_command (unixctl.c:308)
==4463==    by 0x533730: run_connection (unixctl.c:342)
==4463==    by 0x533730: unixctl_server_run (unixctl.c:393)
==4463==    by 0x4073AE: main (ovs-vswitchd.c:128)

Signed-off-by: Damijan Skvarc <damjan.skvarc@gmail.com>
---
 lib/vlog.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Comments

Ben Pfaff Oct. 2, 2019, 2:55 p.m. UTC | #1
On Wed, Oct 02, 2019 at 01:44:49PM +0200, Damijan Skvarc wrote:
> memory leak happens in case previously closed log file was reopened again,
> for example:
> 
> ovs-appctl vlog/close
> ovs-appctl vlog/reopen

Thanks.  I applied this to master.

I removed the 'if' here, since free(NULL) is a no-op:
> +    if (log_file_name) {
> +        free(log_file_name);
> +    }
Damijan Skvarc Oct. 3, 2019, 8:19 a.m. UTC | #2
Perfect!

Thanks.

On Wed, Oct 2, 2019 at 5:39 PM Ben Pfaff <blp@ovn.org> wrote:

> On Wed, Oct 02, 2019 at 01:44:49PM +0200, Damijan Skvarc wrote:
> > memory leak happens in case previously closed log file was reopened
> again,
> > for example:
> >
> > ovs-appctl vlog/close
> > ovs-appctl vlog/reopen
>
> Thanks.  I applied this to master.
>
> I removed the 'if' here, since free(NULL) is a no-op:
> > +    if (log_file_name) {
> > +        free(log_file_name);
> > +    }
>

Patch
diff mbox series

diff --git a/lib/vlog.c b/lib/vlog.c
index 01cfdc5..278b01c 100644
--- a/lib/vlog.c
+++ b/lib/vlog.c
@@ -395,11 +395,14 @@  vlog_set_log_file(const char *file_name)
     /* Close old log file, if any, and install new one. */
     ovs_mutex_lock(&log_file_mutex);
     if (log_fd >= 0) {
-        free(log_file_name);
         close(log_fd);
         async_append_destroy(log_writer);
     }
 
+    if (log_file_name) {
+        free(log_file_name);
+    }
+
     log_file_name = xstrdup(new_log_file_name);
     log_fd = new_log_fd;
     if (log_async) {