[Bug 609473] GstMiniObject derived classes could support storing data for bindings

[GStreamer (bugzilla.gnome.org)]

https://bugzilla.gnome.org/show_bug.cgi?id=609473
  GStreamer | gstreamer (core) | git

Sebastian Dröge <slomo> changed:

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

--- Comment #41 from Sebastian Dröge <slomo at circular-chaos.org> 2011-05-09 08:19:23 UTC ---
Review of attachment 187477:
 --> (https://bugzilla.gnome.org/review?bug=609473&attachment=187477)

Looks good in general, just some minor comments and then it can IMHO be
committed to 0.10... but I'd also like to wait for a second oppinion.

::: gst/gstminiobject.c
@@ +417,3 @@
+ * to stay alive).
+ *
+ * Since: 0.10.33

0.10.34

@@ +430,3 @@
+  g_return_if_fail (GST_MINI_OBJECT_REFCOUNT_VALUE (object) >= 1);
+
+  G_LOCK (weak_refs_mutex);

Instead of the mutex maybe some kind of RCU could be used here? But the mutex
is fine as it's only really used when adding or removing weak refs

@@ +436,3 @@
+    wstack =
+        g_realloc (wstack,
+        sizeof (*wstack) + sizeof (wstack->weak_refs[0]) * i);

You should probably check here if the notify/data combination already exists as
a weak ref

::: tests/check/gst/gstminiobject.c
@@ +183,3 @@
+on_weak_ref_notify (gpointer data, GstMiniObject * where_object_was)
+{
+  weak_ref_notify_succeeded = TRUE;

add a "fail_unless (data == (gpointer) where_object_was)" here

-- 
Configure bugmail: https://bugzilla.gnome.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.