hyperrealm / libconfig

C/C++ library for processing configuration files
https://hyperrealm.github.io/libconfig/
GNU Lesser General Public License v2.1
1.1k stars 360 forks source link

Memory leak caused by invalid syntax #142

Closed mrkiko closed 4 years ago

mrkiko commented 5 years ago

I am using libconfig as a command parser in my program. So, I work with configurations of the following form: AT = (94, "quit") and that's fine. But If I type something like: AT =94, "quit")

then memory leaks starts in libconfig. And processing multiple times the configuration will produce more leaks. I tried to look at the .y code, but I do not understand it yet. Any help would be really apreciated! :)

Enrico

mrkiko commented 4 years ago

any hint on this would be greatly apreciated, Thanks! :)

thomastrapp commented 4 years ago

Interesting use case :) I've tried to reproduce this.Valgrind reports no leaks for this simple program: I can reproduce the issue, when using the config string AT =94, "quit")

#include <stdio.h>
#include <libconfig.h>

int main(int argc, const char * argv[])
{
  config_t cfg;
  config_init(&cfg);

  for(int i = 1; i < argc; i++)
  {
    if( !config_read_string(&cfg, argv[i]) )
      fprintf(stderr, "error: %s\n", config_error_text(&cfg));
  }

  config_destroy(&cfg);
  return 0;
}
gcc -O3 -Wall config_read_string.c -lconfig
valgrind ./a.out 'AT =94, "quit")'
# in use at exit: 64 bytes in 1 blocks
==14127== 64 bytes in 1 blocks are definitely lost in loss record 1 of 1
==14127==    at 0x4C2FA3F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==14127==    by 0x4C31D84: realloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==14127==    by 0x4E44677: strbuf_ensure_capacity (strbuf.c:41)
==14127==    by 0x4E446BF: strbuf_append_string (strbuf.c:59)
==14127==    by 0x4E43355: libconfig_yylex (scanner.l:98)
==14127==    by 0x4E3F90A: libconfig_yyparse (grammar.c:1258)
==14127==    by 0x4E41647: __config_read (libconfig.c:560)
==14127==    by 0x108902: main (config_read_string.c:12)
thomastrapp commented 4 years ago

In your example configure string AT =94, "quit"), the memory for the TOK_STRING "quit" is leaked.

The following patch seems to fix the issue for me. It adds a %destructor for TOK_STRING:

diff --git a/lib/grammar.y b/lib/grammar.y
index 9788ba8..7b31090 100644
--- a/lib/grammar.y
+++ b/lib/grammar.y
@@ -85,6 +85,7 @@ void libconfig_yyerror(void *scanner, struct parse_context *ctx,
 %token <fval> TOK_FLOAT
 %token <sval> TOK_STRING TOK_NAME
 %token TOK_EQUALS TOK_NEWLINE TOK_ARRAY_START TOK_ARRAY_END TOK_LIST_START TOK_LIST_END TOK_COMMA TOK_GROUP_START TOK_GROUP_END TOK_SEMICOLON TOK_GARBAGE TOK_ERROR
+%destructor { free($$); } TOK_STRING

 %%

If I understand correctly, the destructor is called when a TOK_STRING is discarded during error recovery.

mrkiko commented 4 years ago

Hello!!

thank you very very much guys for your help and work; sorry for me taking so long to answer you! Yes - this patch ixes the issue, and I am very thankful for that.

BTW - you can look at the code where I am using this: https://github.com/mrkiko/agh

and in particular, agh_commands.c, function struct agh_cmd agh_text_to_cmd(gchar from, gchar *content);

thank you for all your work.

Enrico

hyperrealm commented 4 years ago

Thomas, could you make a pull request with your patch? I'll merge it in.

thomastrapp commented 4 years ago

@hyperrealm Yes. It will be ready shortly.