Merge branch 'wip/3v1n0/gi-repository-leak-fixes' into 'main'

girepository/build: Actually use our compiler to generate GLib typelibs and fix parser leaks

See merge request GNOME/glib!4064

girepository/compiler/compiler.c

@@ -218,6 +218,7 @@ main (int argc, char **argv)
       char *message = g_strdup_printf (_("Error parsing file ‘%s’: %s"), input[0], error->message);
       g_fprintf (stderr, "%s\n", message);
       g_free (message);
+      gi_ir_parser_free (parser);
 
       return 1;
     }
@@ -253,10 +254,7 @@ main (int argc, char **argv)
 
   g_debug ("[building] done");
 
-#if 0
-  /* No point */
   gi_ir_parser_free (parser);
-#endif
 
   return 0;
 }

girepository/girmodule-private.h

@@ -47,7 +47,7 @@ struct _GIIrModule
   char  *version;
   char  *shared_library;
   char  *c_prefix;
-  GList *dependencies;
+  GPtrArray *dependencies; /* (owned) */
   GList *entries;
 
   /* All modules that are included directly or indirectly */

girepository/girmodule.c

@@ -75,12 +75,15 @@ gi_ir_module_free (GIIrModule *module)
   GList *e;
 
   g_free (module->name);
+  g_free (module->version);
+  g_free (module->shared_library);
+  g_free (module->c_prefix);
 
   for (e = module->entries; e; e = e->next)
     gi_ir_node_free ((GIIrNode *)e->data);
 
   g_list_free (module->entries);
-  /* Don't free dependencies, we inherit that from the parser */
+  g_clear_pointer (&module->dependencies, g_ptr_array_unref);
 
   g_list_free (module->include_modules);
 
@@ -348,35 +351,39 @@ gi_ir_module_build_typelib (GIIrModule *module)
 
   /* Serialize dependencies into one string; this is convenient
    * and not a major change to the typelib format. */
-  {
+  if (module->dependencies->len)
-    GString *dependencies_str = g_string_new ("");
+    {
-    GList *link;
+      GString *dependencies_str = g_string_new (NULL);
-    for (link = module->dependencies; link; link = link->next)
+      for (guint i = module->dependencies->len; i > 0; --i)
-      {
+        {
-        const char *dependency = link->data;
+          const char *dependency = g_ptr_array_index (module->dependencies, i-1);
-        if (!strcmp (dependency, module->name))
+          if (!strcmp (dependency, module->name))
-          continue;
+            continue;
-        g_string_append (dependencies_str, dependency);
+          g_string_append (dependencies_str, dependency);
-        if (link->next)
+          if (i > 1)
-          g_string_append_c (dependencies_str, '|');
+            g_string_append_c (dependencies_str, '|');
-      }
+        }
-    dependencies = g_string_free (dependencies_str, FALSE);
+      dependencies = g_string_free (dependencies_str, FALSE);
-    if (!dependencies[0])
+      if (dependencies && !dependencies[0])
-      {
+        {
-        g_free (dependencies);
+          g_free (dependencies);
-        dependencies = NULL;
+          dependencies = NULL;
-      }
+        }
-  }
+    }
+  else
+    {
+      dependencies = NULL;
+    }
 
  restart:
   gi_ir_node_init_stats ();
   strings = g_hash_table_new (g_str_hash, g_str_equal);
-  types = g_hash_table_new (g_str_hash, g_str_equal);
+  types = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, NULL);
   nodes_with_attributes = NULL;
   n_entries = g_list_length (module->entries);
 
-  g_message ("%d entries (%d local), %d dependencies", n_entries, n_local_entries,
+  g_message ("%d entries (%d local), %u dependencies", n_entries, n_local_entries,
-             g_list_length (module->dependencies));
+             module->dependencies ? module->dependencies->len : 0);
 
   dir_size = n_entries * sizeof (DirEntry);
   size = header_size + dir_size;
@@ -478,7 +485,6 @@ gi_ir_module_build_typelib (GIIrModule *module)
 
   for (e = module->entries, i = 0; e; e = e->next, i++)
     {
-      GIIrTypelibBuild build;
       GIIrNode *node = e->data;
 
       if (strchr (node->name, '.'))
@@ -521,6 +527,7 @@ gi_ir_module_build_typelib (GIIrModule *module)
         }
       else
         {
+          GIIrTypelibBuild build = {0};
           old_offset = offset;
           offset2 = offset + gi_ir_node_get_size (node);
 
@@ -529,7 +536,6 @@ gi_ir_module_build_typelib (GIIrModule *module)
           entry->offset = offset;
           entry->name = gi_ir_write_string (node->name, strings, data, &offset2);
 
-          memset (&build, 0, sizeof (build));
           build.module = module;
           build.strings = strings;
           build.types = types;
@@ -537,6 +543,7 @@ gi_ir_module_build_typelib (GIIrModule *module)
           build.n_attributes = header->n_attributes;
           build.data = data;
           gi_ir_node_build_typelib (node, NULL, &build, &offset, &offset2, NULL);
+          g_clear_list (&build.stack, NULL);
 
           nodes_with_attributes = build.nodes_with_attributes;
           header->n_attributes = build.n_attributes;
@@ -589,6 +596,7 @@ gi_ir_module_build_typelib (GIIrModule *module)
   g_hash_table_destroy (strings);
   g_hash_table_destroy (types);
   g_list_free (nodes_with_attributes);
+  g_free (dependencies);
 
   return typelib;
 }

girepository/girnode.c

@@ -415,10 +415,8 @@ gi_ir_node_free (GIIrNode *node)
         g_free (union_->free_func);
 
         gi_ir_node_free ((GIIrNode *)union_->discriminator_type);
-        for (l = union_->members; l; l = l->next)
+        g_clear_list (&union_->members, (GDestroyNotify) gi_ir_node_free);
-          gi_ir_node_free ((GIIrNode *)l->data);
+        g_clear_list (&union_->discriminators, (GDestroyNotify) gi_ir_node_free);
-        for (l = union_->discriminators; l; l = l->next)
-          gi_ir_node_free ((GIIrNode *)l->data);
       }
       break;
 
@@ -1460,24 +1458,23 @@ gi_ir_node_build_typelib (GIIrNode         *node,
         else
           {
             GString *str;
-            char *s;
             gpointer value;
 
             str = g_string_new (0);
             serialize_type (build, type, str);
-            s = g_string_free (str, FALSE);
 
             types_count += 1;
-            value = g_hash_table_lookup (types, s);
+            value = g_hash_table_lookup (types, str->str);
             if (value)
               {
                 blob->offset = GPOINTER_TO_UINT (value);
-                g_free (s);
+                g_string_free (g_steal_pointer (&str), TRUE);
               }
             else
               {
                 unique_types_count += 1;
-                g_hash_table_insert (types, s, GUINT_TO_POINTER(*offset2));
+                g_hash_table_insert (types, g_string_free_and_steal (g_steal_pointer (&str)),
+                                     GUINT_TO_POINTER(*offset2));
 
                 blob->offset = *offset2;
                 switch (type->tag)

girepository/girparser.c

@@ -120,7 +120,7 @@ struct _ParseContext
 
   GList *modules;
   GList *include_modules;
-  GList *dependencies;
+  GPtrArray *dependencies;
   GHashTable *aliases;
   GHashTable *disguised_structures;
   GHashTable *pointer_structures;
@@ -212,13 +212,10 @@ gi_ir_parser_set_debug (GIIrParser     *parser,
 void
 gi_ir_parser_free (GIIrParser *parser)
 {
-  GList *l;
-
   g_strfreev (parser->includes);
   g_strfreev (parser->gi_gir_path);
 
-  for (l = parser->parsed_modules; l; l = l->next)
+  g_clear_list (&parser->parsed_modules, (GDestroyNotify) gi_ir_module_free);
-    gi_ir_module_free (l->data);
 
   g_slice_free (GIIrParser, parser);
 }
@@ -1353,8 +1350,6 @@ start_parameter (GMarkupParseContext  *context,
   param->closure = closure ? atoi (closure) : -1;
   param->destroy = destroy ? atoi (destroy) : -1;
 
-  ((GIIrNode *)param)->name = g_strdup (name);
-
   switch (CURRENT_NODE (ctx)->type)
     {
     case GI_IR_NODE_FUNCTION:
@@ -3106,9 +3101,8 @@ start_element_handler (GMarkupParseContext  *context,
               return;
             }
 
-          ctx->dependencies = g_list_prepend (ctx->dependencies,
+          g_ptr_array_insert (ctx->dependencies, 0,
-                                              g_strdup_printf ("%s-%s", name, version));
+                              g_strdup_printf ("%s-%s", name, version));
-
 
           state_switch (ctx, STATE_INCLUDE);
           goto out;
@@ -3196,7 +3190,12 @@ start_element_handler (GMarkupParseContext  *context,
               ctx->include_modules = NULL;
 
               ctx->modules = g_list_append (ctx->modules, ctx->current_module);
-              ctx->current_module->dependencies = ctx->dependencies;
+
+              if (ctx->current_module->dependencies != ctx->dependencies)
+                {
+                  g_clear_pointer (&ctx->current_module->dependencies, g_ptr_array_unref);
+                  ctx->current_module->dependencies = g_ptr_array_ref (ctx->dependencies);
+                }
 
               state_switch (ctx, STATE_NAMESPACE);
               goto out;
@@ -3378,13 +3377,19 @@ state_switch_end_struct_or_union (GMarkupParseContext  *context,
                                   const char           *element_name,
                                   GError              **error)
 {
-  pop_node (ctx);
+  GIIrNode *node = pop_node (ctx);
+
   if (ctx->node_stack == NULL)
     {
       state_switch (ctx, STATE_NAMESPACE);
     }
   else
     {
+      /* In this case the node was not tracked by any other node, so we need
+       * to free the node, or we'd leak.
+       */
+      g_clear_pointer (&node, gi_ir_node_free);
+
       if (CURRENT_NODE (ctx)->type == GI_IR_NODE_STRUCT)
         state_switch (ctx, STATE_STRUCT);
       else if (CURRENT_NODE (ctx)->type == GI_IR_NODE_UNION)
@@ -3736,6 +3741,8 @@ cleanup (GMarkupParseContext *context,
   ParseContext *ctx = user_data;
   GList *m;
 
+  g_clear_slist (&ctx->node_stack, NULL);
+
   for (m = ctx->modules; m; m = m->next)
     gi_ir_module_free (m->data);
   g_list_free (ctx->modules);
@@ -3770,6 +3777,7 @@ gi_ir_parser_parse_string (GIIrParser   *parser,
 {
   ParseContext ctx = { 0 };
   GMarkupParseContext *context;
+  GIIrModule *module = NULL;
 
   ctx.parser = parser;
   ctx.state = STATE_START;
@@ -3780,7 +3788,7 @@ gi_ir_parser_parse_string (GIIrParser   *parser,
   ctx.disguised_structures = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, NULL);
   ctx.pointer_structures = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, NULL);
   ctx.type_depth = 0;
-  ctx.dependencies = NULL;
+  ctx.dependencies = g_ptr_array_new_with_free_func (g_free);
   ctx.current_module = NULL;
 
   context = g_markup_parse_context_new (&firstpass_parser, 0, &ctx, NULL);
@@ -3801,12 +3809,15 @@ gi_ir_parser_parse_string (GIIrParser   *parser,
   if (!g_markup_parse_context_end_parse (context, error))
     goto out;
 
-  parser->parsed_modules = g_list_concat (g_list_copy (ctx.modules),
+  if (ctx.modules)
+    module = ctx.modules->data;
+
+  parser->parsed_modules = g_list_concat (g_steal_pointer (&ctx.modules),
                                           parser->parsed_modules);
 
  out:
 
-  if (ctx.modules == NULL)
+  if (module == NULL)
     {
       /* An error occurred before we created a module, so we haven't
        * transferred ownership of these hash tables to the module.
@@ -3814,13 +3825,16 @@ gi_ir_parser_parse_string (GIIrParser   *parser,
       g_clear_pointer (&ctx.aliases, g_hash_table_unref);
       g_clear_pointer (&ctx.disguised_structures, g_hash_table_unref);
       g_clear_pointer (&ctx.pointer_structures, g_hash_table_unref);
+      g_clear_list (&ctx.modules, (GDestroyNotify) gi_ir_module_free);
       g_list_free (ctx.include_modules);
     }
 
+  g_clear_slist (&ctx.node_stack, NULL);
+  g_clear_pointer (&ctx.dependencies, g_ptr_array_unref);
   g_markup_parse_context_free (context);
 
-  if (ctx.modules)
+  if (module)
-    return ctx.modules->data;
+    return module;
 
   if (error && *error == NULL)
     g_set_error (error,

girepository/meson.build

@@ -240,6 +240,8 @@ pkg.generate(libgirepository,
   libraries: [libglib_dep, libgobject_dep],
 )
 
+subdir('compiler')
+
 if enable_gir
   subdir('introspection')
 endif
@@ -248,6 +250,5 @@ if build_tests
   subdir('tests')
 endif
 
-subdir('compiler')
 subdir('decompiler')
 subdir('inspector')