[ovs-dev,4/4] vlog: Fix memory leak

Message ID 1515779132-28902-4-git-send-email-pkusunyifeng@gmail.com
State Changes Requested
Headers show
Series
  • [ovs-dev,1/4] pinctrl: Fix memory leak
Related show

Commit Message

Yifeng Sun Jan. 12, 2018, 5:45 p.m.
This leak was reported by valgrind (testing vlog - vlog/close - C):

53 bytes in 1 blocks are definitely lost in loss record 60 of 66      
    by 0x4D1214: xmalloc (util.c:120)                                  
    by 0x4D1284: xmemdup0 (util.c:150)                                 
    by 0x4D7AF9: vlog_set_log_file (vlog.c:402)                        
    by 0x42C1AB: parse_options (test-unixctl.c:155)                    
    by 0x42C1AB: test_unixctl_main (test-unixctl.c:84)                 
    by 0x42C1AB: ovstest_wrapper_test_unixctl_main__ (test-unixctl.c:186)
    by 0x43C743: ovs_cmdl_run_command__ (command-line.c:115)           
    by 0x406C09: main (ovstest.c:133)

Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com>
---
 lib/vlog.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Ben Pfaff Jan. 23, 2018, 12:43 a.m. | #1
On Fri, Jan 12, 2018 at 09:45:32AM -0800, Yifeng Sun wrote:
> This leak was reported by valgrind (testing vlog - vlog/close - C):
> 
> 53 bytes in 1 blocks are definitely lost in loss record 60 of 66      
>     by 0x4D1214: xmalloc (util.c:120)                                  
>     by 0x4D1284: xmemdup0 (util.c:150)                                 
>     by 0x4D7AF9: vlog_set_log_file (vlog.c:402)                        
>     by 0x42C1AB: parse_options (test-unixctl.c:155)                    
>     by 0x42C1AB: test_unixctl_main (test-unixctl.c:84)                 
>     by 0x42C1AB: ovstest_wrapper_test_unixctl_main__ (test-unixctl.c:186)
>     by 0x43C743: ovs_cmdl_run_command__ (command-line.c:115)           
>     by 0x406C09: main (ovstest.c:133)
> 
> Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com>

Thanks for the patch.

This changes user-visible behavior because clearing the log file name
means that the log file can't be reopened later, e.g. this test fails:

    1197. ofproto-dpif.at:8255: testing ofproto-dpif - ofproto-dpif-monitor 1 ...
    ...
    ../../tests/ofproto-dpif.at:8266: sed -e '/^.*ofproto_dpif_monitor.*$/d' < ovs-vswitchd.log > tmp && ovs-appctl vlog/close && mv tmp ovs-vswitchd.log && ovs-appctl vlog/reopen
    --- /dev/null   2017-07-26 15:46:07.674034656 -0700
    +++ /home/blp/nicira/ovs/_build/tests/testsuite.dir/at-groups/1197/stderr       2018-01-22 16:38:08.811578159 -0800
    @@ -0,0 +1,2 @@
    +Logging to file not configured
    +ovs-appctl: ovs-vswitchd: server returned an error

Perhaps there is a way to fix it without changing behavior?

Thanks,

Ben.

Patch

diff --git a/lib/vlog.c b/lib/vlog.c
index 6e87665..a5c0c5e 100644
--- a/lib/vlog.c
+++ b/lib/vlog.c
@@ -696,6 +696,8 @@  vlog_unixctl_close(struct unixctl_conn *conn, int argc OVS_UNUSED,
     if (log_fd >= 0) {
         close(log_fd);
         log_fd = -1;
+        free(log_file_name);
+        log_file_name = NULL;
 
         async_append_destroy(log_writer);
         log_writer = NULL;