orangeduck/mpc

memory leak with mpc_check(mpc_apply(…), …)?

nmeum opened this issue · 5 comments

nmeum commented

Consider the following program:

#include <stdio.h>
#include <string.h>

#include "../mpc.h"

static int
check_string(mpc_val_t **val)
{
	char *str;

	str = *(char **)val;
	return !strcmp("foobar", str);
}

int main(void) {
    mpc_result_t r;
    mpc_parser_t* Foobar;

    Foobar = mpc_check(mpc_apply(mpc_noneof(" \t\n"), mpcf_escape), check_string, "invalid string");
    if (mpc_parse("<stdin>", "foo", Foobar, &r)) {
        puts("Success!");
    } else {
        mpc_err_print(r.error);
        mpc_err_delete(r.error);
    }

    mpc_cleanup(1, Foobar);
    return 0;
}

I would not expect it to leave any memory unfreed. However, running this program with valgrind yields an error:

$ valgrind --leak-check=full ./examples/foobar
==18669== Memcheck, a memory error detector
==18669== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==18669== Using Valgrind-3.14.0 and LibVEX; rerun with -h for copyright info
==18669== Command: ./examples/foobar
==18669== 
<stdin>: error: invalid string
==18669== 
==18669== HEAP SUMMARY:
==18669==     in use at exit: 478 bytes in 5 blocks
==18669==   total heap usage: 28 allocs, 23 frees, 38,700 bytes allocated
==18669== 
==18669== 2 bytes in 1 blocks are definitely lost in loss record 1 of 3
==18669==    at 0x489D8C2: realloc (vg_replace_malloc.c:826)
==18669==    by 0x10A39D: mpcf_escape_new (mpc.c:2513)
==18669==    by 0x1126CB: mpcf_escape (mpc.c:2566)
==18669==    by 0x112DD8: mpc_parse_apply (mpc.c:1023)
==18669==    by 0x112DD8: mpc_parse_run (mpc.c:1080)
==18669==    by 0x112F2B: mpc_parse_run (mpc.c:1093)
==18669==    by 0x113B90: mpc_parse_input (mpc.c:1302)
==18669==    by 0x113D3F: mpc_parse (mpc.c:1315)
==18669==    by 0x1092A1: main (foobar.c:21)
==18669== 
==18669== LEAK SUMMARY:
==18669==    definitely lost: 2 bytes in 1 blocks
==18669==    indirectly lost: 0 bytes in 0 blocks
==18669==      possibly lost: 0 bytes in 0 blocks
==18669==    still reachable: 0 bytes in 0 blocks
==18669==         suppressed: 476 bytes in 4 blocks
==18669== 
==18669== For counts of detected and suppressed errors, rerun with: -v
==18669== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)

Which seems unexpected to me.

There does seem to be a workaround: Freeing the mpc_val_t from the check function, e.g.:

--- a/examples/foobar.c
+++ b/examples/foobar.c
@@ -9,6 +9,8 @@ check_string(mpc_val_t **val)
        char *str;
 
        str = *(char **)val;
+       if (strcmp("foobar", str))
+         free(str);
        return !strcmp("foobar", str);
 }

Thanks for the report. I will check it out.

Okay - I think the problem is that you need to free the result of the parse on success. I.E.

if (mpc_parse("<stdin>", "foo", Foobar, &r)) {
    free(r.output);
    puts("Success!");

However I think there is still a problem with the check function. I think it needs to take a destructor function in case the check fails.

Okay I added a destructor to the check function in dbe7308. Now if you add a free just before puts like I suggested, now there should be no memory leaks even in the case where the check fails. Would you be able to verify that for me?

Thanks,

Dan

nmeum commented

Yep, adding a destructor argument to mpc_check fixes the issue. I can no longer reproduce the leak, thanks!