[Bug 765794] registrychunks: Memory leak

[GStreamer (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=765794

Jan Schmidt <thaytan at noraisin.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #327555|none                        |needs-work
             status|                            |

--- Comment #12 from Jan Schmidt <thaytan at noraisin.net> ---
Review of attachment 327555:
 --> (https://bugzilla.gnome.org/review?bug=765794&attachment=327555)

Please expand the commit description a little. Something like:

"registrychunks: Fix memory leaks in error cases

Make sure to free partially allocated memory during error conditions to avoid
leaks"

::: gst/gstregistrychunks.c
@@ +670,3 @@
       for (i = tff->nextensions; i > 0; i--) {
         unpack_string (*in, str, end, fail);
+        typefind_factory->extensions[i - 1] = str;

All these factory to typefind_factory / element_factory changes are unnecessary
and make the diff messy. Since the only explicit cleanup to do is for the
typefind factory,
check GST_IS_TYPE_FIND_FACTORY(feature) in the fail: case and cast the feature
variable there.

@@ +733,3 @@
+
+  if (element_factory->uri_protocols)
+    g_strfreev (element_factory->uri_protocols);

This doesn't seem correct. uri_protocols should not be leaking. The
GstElementFactory already calls g_strfreev in gst_element_factory_cleanup()
called during finalize().

@@ +739,3 @@
+      g_free (typefind_factory->extensions[i]);
+    g_free (typefind_factory->extensions);
+  }

It makes sense to clean up a partially constructed extensions array in this
way, but the typefind_factory will also be calling g_strfreev on that during
finalize, so it needs to be set to NULL, and the cleanup needs to happen
*before* unreffing the factory a few lines above, because at that point it's
hopefully already destroyed.

@@ +759,3 @@
   return arr;
 fail:
+  g_strfreev (arr);

This seems correct

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.