Memory leak - failure to free lexer allocated memory.

[nadav2051]

Hey, I've stumbled across a possible memory leak - not sure whether it's intentional or not -

It appears as if the free_cfg() library function neglects to free the yy_buffer_stack global variable.

I externed the cfg_yylex_destroy() function and used it straight after calling to free_cfg() and the memory leak was gone.

[brankod]

This is still present in 3.0 and current git version.

==2644== 8 bytes in 1 blocks are still reachable in loss record 1 of 1
==2644== at 0x4C2FB55: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==2644== by 0x42FF5E: cfg_yyalloc (lexer.c:2288)
==2644== by 0x42FF5E: cfg_yyensure_buffer_stack (lexer.c:1986)
==2644== by 0x42FF98: cfg_yy_switch_to_buffer (lexer.c:1785)
==2644== by 0x42ED5E: cfg_parse_fp (confuse.c:1290)
==2644== by 0x42EE3D: cfg_parse (confuse.c:1389)

[troglobit]

Great! Could you prepare a pull request?

[brankod]

Sorry, I don't have a fix (yet, anyway). I'll try what nadav2051 proposed and if it works, i'll share it.

[troglobit]

@brankod Looking forward to it! 😃

[brankod]

I looked into this and I'm not sure there is an easy fix. As nadav2051 reports, calling cfg_yylex_destroy() after all parsing is done, frees yy_buffer_stack. I hoped this could be added to cfg_free, but after this change tests that call cfg_free/cfgf_parse multiple times fail. I'll scratch my head a bit more, why.

[troglobit]

Thanks, I've noticed this too. Don't worry, I'll look into it during the weekend ☺👍

[brankod]

I was thinking, one solution is to simply add a public function that calls cfg_yylex_destroy(), possibly marked with attribute((destructor)). It is really unimportant leak most users probably don't care about, but some of us like too see valgrind report "no leaks possible". :) What do you think?

[troglobit]

I was actually thinking the same thing, so in my tree I've added a cfg_exit(), to pair with cfg_init(), which calls cfg_yylex_destroy(). Didn't consider attribute((destructor)) though, that was clever!

However, as I've been going through regressions in test cases, with the previous idea for a fix, I noticed there are a few annoying leaks that are aggregated every time cfg_parse_buf() is called. I'm currently investigating this.

[troglobit]

Turned out to be quite a large change set to address all the memory leaks I encountered while fixing this. Fortunately the changes made do not introduce any new API, cfg_free() can now call cfg_yylex_destroy() to free all the memory allocated by the scanner.

It would be most helpful to get feedback/review of pull request #75 ... if you do, please use the GitHub review feature (top right corner of the PR).