diff mbox

[ovs-dev,3/3] ofproto: fix memory leak reported by valgrind

Message ID 1452029923-8685-3-git-send-email-u9012063@gmail.com
State Accepted
Headers show

Commit Message

William Tu Jan. 5, 2016, 9:38 p.m. UTC 
Test case 757: ofproto - table description (OpenFlow 1.4)
Call stacks:
    parse_ofp_table_vacancy (ofp-parse.c:896)
    parse_ofp_table_mod (ofp-parse.c:978)
    ofctl_mod_table (ovs-ofctl.c:2011)
    ovs_cmdl_run_command (command-line.c:121)
    main (ovs-ofctl.c:135)
Reason: return without freeing memory

Signed-off-by: William Tu <u9012063@gmail.com>
Signed-off-by: Daniele Di Proietto <diproiettod@vmware.com>
Co-authored-by: Daniele Di Proietto <diproiettod@vmware.com>
---
 lib/ofp-parse.c | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

Comments

Ben Pfaff Jan. 11, 2016, 5:21 p.m. UTC | #1 
On Tue, Jan 05, 2016 at 01:38:43PM -0800, William Tu wrote:
> Test case 757: ofproto - table description (OpenFlow 1.4)
> Call stacks:
>     parse_ofp_table_vacancy (ofp-parse.c:896)
>     parse_ofp_table_mod (ofp-parse.c:978)
>     ofctl_mod_table (ovs-ofctl.c:2011)
>     ovs_cmdl_run_command (command-line.c:121)
>     main (ovs-ofctl.c:135)
> Reason: return without freeing memory
> 
> Signed-off-by: William Tu <u9012063@gmail.com>
> Signed-off-by: Daniele Di Proietto <diproiettod@vmware.com>
> Co-authored-by: Daniele Di Proietto <diproiettod@vmware.com>

Thanks, applied to master and branch-2.5.
diff mbox

Patch

diff --git a/lib/ofp-parse.c b/lib/ofp-parse.c
index aa0cad7..e8e09db 100644
--- a/lib/ofp-parse.c
+++ b/lib/ofp-parse.c
@@ -894,33 +894,45 @@  parse_ofp_table_vacancy(struct ofputil_table_mod *tm, const char *setting)
     char *save_ptr = NULL;
     char *vac_up, *vac_down;
     char *value = strdup(setting);
+    char *ret_msg;
     int vacancy_up, vacancy_down;
 
     strtok_r(value, ":", &save_ptr);
     vac_down = strtok_r(NULL, ",", &save_ptr);
     if (!vac_down) {
-        return xasprintf("Vacancy down value missing");
+        ret_msg = xasprintf("Vacancy down value missing");
+        goto exit;
     }
     if (!str_to_int(vac_down, 0, &vacancy_down) ||
         vacancy_down < 0 || vacancy_down > 100) {
-        return xasprintf("Invalid vacancy down value \"%s\"", vac_down);
+        ret_msg = xasprintf("Invalid vacancy down value \"%s\"", vac_down);
+        goto exit;
     }
     vac_up = strtok_r(NULL, ",", &save_ptr);
     if (!vac_up) {
-        return xasprintf("Vacancy up value missing");
+        ret_msg = xasprintf("Vacancy up value missing");
+        goto exit;
     }
     if (!str_to_int(vac_up, 0, &vacancy_up) ||
         vacancy_up < 0 || vacancy_up > 100) {
-        return xasprintf("Invalid vacancy up value \"%s\"", vac_up);
+        ret_msg = xasprintf("Invalid vacancy up value \"%s\"", vac_up);
+        goto exit;
     }
     if (vacancy_down > vacancy_up) {
-        return xasprintf("Invalid vacancy range, vacancy up should be greater"
+        ret_msg = xasprintf("Invalid vacancy range, vacancy up should be greater"
                          " than vacancy down ""(%s)",
                          ofperr_to_string(OFPERR_OFPBPC_BAD_VALUE));
+        goto exit;
     }
+
+    free(value);
     tm->table_vacancy.vacancy_down = vacancy_down;
     tm->table_vacancy.vacancy_up = vacancy_up;
     return NULL;
+
+exit:
+    free(value);
+    return ret_msg;
 }
 
 /* Convert 'table_id' and 'setting' (as described for the "mod-table" command