[gstreamermm] Comment on TagList code and avoid some warnings

[Daniel Elstner <daniel src gnome org>]

commit 7ac8e5808fd9bed9d8c0a2687bb57e014d1125a9
Author: Daniel Elstner <danielk openismus com>
Date:   Tue Oct 6 13:51:13 2009 +0200

    Comment on TagList code and avoid some warnings
    
    * tools/m4/class_boxedtype_extra.m4 (_END_CLASS_BOXEDTYPE_EXTRA):
    Remove dummy parameter name from the definition of Glib::wrap().
    Comment on the dummy parameter looking fishy to me.
    * gstreamer/src/taglist.hg (TagList): Explain in a comment that the
    class is wrapped incorrectly and should be reworked at the next ABI
    break.
    * gstreamer/src/taglist.ccg: Place file-scope declarations into an
    anonymous namespace.
    (TagList_foreach_gstreamermm_callback): Remove unused parameter name
    to get rid of a compiler warning.  Use extern "C" calling convention.
    (TagList::add_value): Call new gst_tag_list_add_value() function.
    (TagList::add): Replace C-style casts with appropriate C++ casts.

 ChangeLog                         |   17 +++++++++++++++
 gstreamer/src/taglist.ccg         |   40 ++++++++++++++++++++++--------------
 gstreamer/src/taglist.hg          |    5 ++++
 tools/m4/class_boxedtype_extra.m4 |    3 +-
 4 files changed, 48 insertions(+), 17 deletions(-)
---
diff --git a/ChangeLog b/ChangeLog
index 7fe4b31..bc6f5fd 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,22 @@
 2009-10-06  Daniel Elstner  <danielk openismus com>
 
+	Comment on TagList code and avoid some warnings
+
+	* tools/m4/class_boxedtype_extra.m4 (_END_CLASS_BOXEDTYPE_EXTRA):
+	Remove dummy parameter name from the definition of Glib::wrap().
+	Comment on the dummy parameter looking fishy to me.
+	* gstreamer/src/taglist.hg (TagList): Explain in a comment that the
+	class is wrapped incorrectly and should be reworked at the next ABI
+	break.
+	* gstreamer/src/taglist.ccg: Place file-scope declarations into an
+	anonymous namespace.
+	(TagList_foreach_gstreamermm_callback): Remove unused parameter name
+	to get rid of a compiler warning.  Use extern "C" calling convention.
+	(TagList::add_value): Call new gst_tag_list_add_value() function.
+	(TagList::add): Replace C-style casts with appropriate C++ casts.
+
+2009-10-06  Daniel Elstner  <danielk openismus com>
+
 	Clean up RingBuffer code and avoid warning
 
 	* gstreamer/src/ringbuffer.hg: Annotate API with a few TODO comments.
diff --git a/gstreamer/src/taglist.ccg b/gstreamer/src/taglist.ccg
index a2f0c10..4915547 100644
--- a/gstreamer/src/taglist.ccg
+++ b/gstreamer/src/taglist.ccg
@@ -19,29 +19,38 @@
 
 #include <gst/gstenumtypes.h>
 
-static void TagList_foreach_gstreamermm_callback(const GstTagList* list, const gchar *tag, void* data)
+namespace
 {
-  Gst::TagList::SlotForeach* slot = static_cast<Gst::TagList::SlotForeach*>(data);
+extern "C"
+{
+
+static void TagList_foreach_gstreamermm_callback(const GstTagList*, const char *tag, void* data)
+{
+  Gst::TagList::SlotForeach& slot_foreach = *static_cast<Gst::TagList::SlotForeach*>(data);
 
-  const Glib::ustring tag_str = Glib::convert_const_gchar_ptr_to_ustring(tag);
-  #ifdef GLIBMM_EXCEPTIONS_ENABLED
+#ifdef GLIBMM_EXCEPTIONS_ENABLED
   try
+#endif
   {
-  #endif //GLIBMM_EXCEPTIONS_ENABLED
-    (*slot)(tag_str);
-  #ifdef GLIBMM_EXCEPTIONS_ENABLED
+    slot_foreach(Glib::convert_const_gchar_ptr_to_ustring(tag));
   }
-  catch(...)
+#ifdef GLIBMM_EXCEPTIONS_ENABLED
+  catch (...)
   {
     Glib::exception_handlers_invoke();
   }
-  #endif //GLIBMM_EXCEPTIONS_ENABLED
+#endif
 }
 
+} // extern "C"
+} // anonymous namespace
+
 namespace Gst
 {
 
 // Make sure the order here is the same order as in Gst:Tag.
+// TODO: It would probably be better to export a lookup function instead of
+// the array itself.
 const char* const _tag_strings[] =
 {
   GST_TAG_TITLE,
@@ -111,9 +120,8 @@ void TagList::add_value(Tag tag, const Glib::ValueBase& value, TagMergeMode mode
 
 void TagList::add_value(const Glib::ustring& tag, const Glib::ValueBase& value, TagMergeMode mode)
 {
-  //TODO: The gst_tag_list_add_values() documentation says nothing about ending the ... with NULL.
-  gst_tag_list_add_values(gobj(), (GstTagMergeMode) mode, tag.c_str(),
-    value.gobj(), (void*)0);
+  gst_tag_list_add_value(gobj(), static_cast<GstTagMergeMode>(mode),
+                         tag.c_str(), value.gobj());
 }
 
 void TagList::add(Tag tag, const char* data, TagMergeMode mode)
@@ -123,14 +131,14 @@ void TagList::add(Tag tag, const char* data, TagMergeMode mode)
 
 void TagList::add(const Glib::ustring& tag, const char* data, TagMergeMode mode)
 {
-  gst_tag_list_add(gobj(), (GstTagMergeMode) mode, tag.c_str(), data,
-    (void*)0);
+  gst_tag_list_add(gobj(), static_cast<GstTagMergeMode>(mode), tag.c_str(), data,
+                   static_cast<void*>(0));
 }
 
 void TagList::foreach(const SlotForeach& slot)
 {
   gst_tag_list_foreach(gobj(), &TagList_foreach_gstreamermm_callback,
-    const_cast<SlotForeach*>(&slot));
+                       const_cast<SlotForeach*>(&slot));
 }
 
 bool TagList::get_value(Tag tag, Glib::ValueBase& dest) const
@@ -163,4 +171,4 @@ bool TagList::get_value(const Glib::ustring& tag, guint index, Glib::ValueBase&
   return false;
 }
 
-} //namespace Gst
+} // namespace Gst
diff --git a/gstreamer/src/taglist.hg b/gstreamer/src/taglist.hg
index f4c4ff2..a349705 100644
--- a/gstreamer/src/taglist.hg
+++ b/gstreamer/src/taglist.hg
@@ -324,6 +324,11 @@ extern const char* const _tag_strings[];
  */
 class TagList : public Structure
 {
+  // TODO: Gst::Structure is already wrapped as a boxed type.  Wrapping this
+  // derived class as a boxed type again is wrong.  It even means that there
+  // are *two* separate gobject_ members!  The compiler warning about the
+  // missing explicit base instance initialization in the copy constructor
+  // exposes the problem, too.
   _CUSTOM_STRUCT_PROTOTYPE
   _CLASS_BOXEDTYPE_EXTRA(TagList, GstTagList, gst_tag_list_new, gst_tag_list_copy, gst_tag_list_free)
   _IGNORE(gst_tag_list_copy, gst_tag_list_free)
diff --git a/tools/m4/class_boxedtype_extra.m4 b/tools/m4/class_boxedtype_extra.m4
index e4a1949..f52dc28 100644
--- a/tools/m4/class_boxedtype_extra.m4
+++ b/tools/m4/class_boxedtype_extra.m4
@@ -66,6 +66,7 @@ ifdef(`__BOOL_NO_WRAP_FUNCTION__',`dnl
 ',`dnl else
 
 /** A Glib::wrap() method for this object. The dummy int parameter is added to disambiguate Gst::TagList::wrap() from Gst::Structure::wrap() (GstTagList is in fact a GstStructure so wrap method becomes ambiguous).
+dnl TODO: This seems like a bad idea to me.  Why not just rename the function?
  * 
  * @param object The C instance.
  * @param dummy An unused parameter to disambiguate Gst::TagList::wrap() from Gst::Structure::wrap().  The value of this parameter is irrelevant.
@@ -92,7 +93,7 @@ ifdef(`__BOOL_NO_WRAP_FUNCTION__',`dnl
 namespace Glib
 {
 
-__NAMESPACE__::__CPPNAME__ wrap(__CNAME__* object, int dummy, bool take_copy)
+__NAMESPACE__::__CPPNAME__ wrap(__CNAME__* object, int, bool take_copy)
 {
   return __NAMESPACE__::__CPPNAME__`'(object, take_copy);
 }