Created attachment 185322 [details]
Test-case for g_option_context_parse()

If a option is specified twice on the commandline and a unknown option is specified like:

  $ prg --known=foo --known=bar --unknown=baz

the arg_data of the "known" option isn't left unchanged. Instead it contains a pointer to free()d memory:

==53227== Invalid free() / delete / delete[]
==53227==    at 0x10000FC7C: free (vg_replace_malloc.c:366)
==53227==    by 0x100000DC1: main (goption-cli.c:46)
==53227==  Address 0x1056dd870 is 0 bytes inside a block of size 7 free'd
==53227==    at 0x10000FC7C: free (vg_replace_malloc.c:366)
==53227==    by 0x10005A623: parse_arg (in /opt/local/lib/libglib-2.0.0.dylib)
==53227==    by 0x10005AA56: parse_long_option (in /opt/local/lib/libglib-2.0.0.dylib)
==53227==    by 0x10005B553: g_option_context_parse (in /opt/local/lib/libglib-2.0.0.dylib)
==53227==    by 0x100000D77: main (goption-cli.c:34)

The attached testcase shows the faulty behaviour.

  $ gcc -Wall -O2 -g `pkg-config glib-2.0 --libs --cflags` goption-cli.c -o goption-cli 
  $ valgrind goption-cli

The problem is in goption.c: parse_arg() for G_OPTION_ARG_FILENAME that does:

 /* first value */
 data = g_strdup (value);
 change = get_change (context, G_OPTION_ARG_FILENAME, entry->arg_data);
 change->prev.str = *(gchar **)entry->arg_data;   /* sets the original value */
 g_free (change->allocated.str); /* noop */
 change->allocated.str = data;
 *(gchar **)entry->arg_data = data;

 /* second value */
 data = g_strdup (value);
 change = get_change (context, G_OPTION_ARG_FILENAME, entry->arg_data); /* returns the same "Change" */
 change->prev.str = *(gchar **)entry->arg_data;      /* points to the 'data' of the previous round */
 g_free (change->allocated.str);       /* BUG: free's the previous data, makes change->prev.str pointing to a invalid location */
 change->allocated.str = data;
 *(gchar **)entry->arg_data = data;

in free_changes_list() it does:

 g_free (change->allocated.str);
 *(gchar **)change->arg_data = change->prev.str; /* change->prev.str is already invalid */

G_OPTION_ARG_STRING has the same problem.

Instead of setting change->prev.str each time, it should only be set for the first time which change is used. A possible fix could be:

--- glib-2.28.5/glib/goption.c  2011-03-04 18:50:39.000000000 +0100
+++ glib-2.28.5-goption-bug/glib/goption.c      2011-04-06 16:19:04.000000000 +0200
@@ -1212,9 +1212,12 @@
 #endif
         change = get_change (context, G_OPTION_ARG_FILENAME,
                              entry->arg_data);
+       if (NULL == change->allocated.str) {
+          /* only store the original value of arg_data */
+          change->prev.str = *(gchar **)entry->arg_data;
+        }
         g_free (change->allocated.str);
 
-        change->prev.str = *(gchar **)entry->arg_data;
         change->allocated.str = data;
 
         *(gchar **)entry->arg_data = data;