GNOME.org
 

[cluttermm] Correct bad code pointed out by compiler warnings

commit 185711fc58689f0bf1ad38cb5d64c1a5a5a06a94
Author: Daniel Elstner <danielk openismus com>
Date:   Thu Oct 1 21:55:33 2009 +0200

    Correct bad code pointed out by compiler warnings
    
    * clutter/src/timeline.{ccg,hg} (Timeline::Timeline): Implement the
    constructor manually, because the parameter names do not match the
    names of the corresponding properties.  Also, adjust the signature to
    the new ClutterTimeline API.
    (Timeline::create): Change signature to match the constructor.
    * clutter/cluttermm/{frame-source,threads}.cc: Add missing inline and
    static keywords, and slightly clean up the code.
    (SourceConnectionNode::notify): Actually return a defined value.
    * examples/test-actors.cc (main), tests/test-alpha-creation.cc (main):
    Adjust for the new Clutter::Timeline::create() API.
    * tests/test-alpha-func.cc (on_alpha): Remove unused parameter name.
    (main): Adapt to Clutter::Timeline::create() API change.

 ChangeLog                         |   17 +++++++++++
 clutter/cluttermm/frame-source.cc |   36 +++++++++++++-----------
 clutter/cluttermm/frame-source.h  |    8 +++---
 clutter/cluttermm/threads.cc      |   55 +++++++++++++++++++++----------------
 clutter/src/timeline.ccg          |   12 +++++---
 clutter/src/timeline.hg           |   15 ++++------
 examples/test-actors.cc           |    2 +-
 tests/test-alpha-creation.cc      |    2 +-
 tests/test-alpha-func.cc          |    6 ++--
 9 files changed, 90 insertions(+), 63 deletions(-)
---
diff --git a/ChangeLog b/ChangeLog
index 843fb6f..e6b9016 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,22 @@
 2009-10-02  Daniel Elstner  <danielk openismus com>
 
+	Correct bad code pointed out by compiler warnings
+
+	* clutter/src/timeline.{ccg,hg} (Timeline::Timeline): Implement the
+	constructor manually, because the parameter names do not match the
+	names of the corresponding properties.  Also, adjust the signature to
+	the new ClutterTimeline API.
+	(Timeline::create): Change signature to match the constructor.
+	* clutter/cluttermm/{frame-source,threads}.cc: Add missing inline and
+	static keywords, and slightly clean up the code.
+	(SourceConnectionNode::notify): Actually return a defined value.
+	* examples/test-actors.cc (main), tests/test-alpha-creation.cc (main):
+	Adjust for the new Clutter::Timeline::create() API.
+	* tests/test-alpha-func.cc (on_alpha): Remove unused parameter name.
+	(main): Adapt to Clutter::Timeline::create() API change.
+
+2009-10-02  Daniel Elstner  <danielk openismus com>
+
 	Adopt new mm-common build infrastructure
 
 	* build/: Rename directory from scripts/ and delete the obsolete
diff --git a/clutter/cluttermm/frame-source.cc b/clutter/cluttermm/frame-source.cc
index 126cfd0..8908cf1 100644
--- a/clutter/cluttermm/frame-source.cc
+++ b/clutter/cluttermm/frame-source.cc
@@ -38,12 +38,14 @@ public:
 
 private:
   sigc::slot_base slot_;
-  guint source_id_;
+  guint           source_id_;
 };
 
+inline
 SourceConnectionNode::SourceConnectionNode(const sigc::slot_base& slot)
 :
-  slot_(slot), source_id_(0)
+  slot_      (slot),
+  source_id_ (0)
 {
   slot_.set_parent(this, &SourceConnectionNode::notify);
 }
@@ -59,61 +61,61 @@ void* SourceConnectionNode::notify(void* data)
     // Removing the source triggers the destroy_notify_handler, wait until
     // that for deletion.
   }
+  return 0;
 }
 
 void SourceConnectionNode::destroy_notify_callback(void* data)
 {
   SourceConnectionNode* const self = static_cast<SourceConnectionNode*>(data);
-  if(self)
-  {
-    self->source_id_ = 0;
-    delete self;
-  }
+  self->source_id_ = 0;
+  delete self;
 }
 
+inline
 void SourceConnectionNode::install(guint source_id)
 {
   source_id_ = source_id;
 }
 
+inline
 sigc::slot_base* SourceConnectionNode::get_slot()
 {
   return &slot_;
 }
 
+static
 gboolean source_callback(void* data)
 {
   SourceConnectionNode* const conn_data = static_cast<SourceConnectionNode*>(data);
-
 #ifdef GLIBMM_EXCEPTIONS_ENABLED
   try
-  {
 #endif
+  {
     // Recreate the specific slot from the generic slot node
     return (*static_cast<sigc::slot<bool>*>(conn_data->get_slot()))();
-#ifdef GLIBMM_EXCEPTIONS_ENABLED
   }
+#ifdef GLIBMM_EXCEPTIONS_ENABLED
   catch(...)
   {
     Glib::exception_handlers_invoke();
   }
-#endif // GLIBMM_EXCEPTIONS_ENABLED
+#endif
   return FALSE;
 }
 
-}
+} // anonymous namespace
 
 namespace Clutter
 {
 
-sigc::connection frame_source_add(const sigc::slot<bool>& callback, guint interval, gint priority)
+sigc::connection frame_source_add(const sigc::slot<bool>& callback, guint interval, int priority)
 {
   SourceConnectionNode* const conn_node = new SourceConnectionNode(callback);
-  const sigc::connection connection(*conn_node->get_slot());
+  const sigc::connection connection (*conn_node->get_slot());
 
-  guint id = clutter_frame_source_add_full(priority, interval, &source_callback, conn_node, &SourceConnectionNode::destroy_notify_callback);
-  conn_node->install(id);
+  conn_node->install(clutter_frame_source_add_full(priority, interval, &source_callback, conn_node,
+                                                   &SourceConnectionNode::destroy_notify_callback));
   return connection;
 }
 
-} //namespace Clutter
+} // namespace Clutter
diff --git a/clutter/cluttermm/frame-source.h b/clutter/cluttermm/frame-source.h
index 1ce3bcc..5a294c9 100644
--- a/clutter/cluttermm/frame-source.h
+++ b/clutter/cluttermm/frame-source.h
@@ -44,9 +44,9 @@ namespace Clutter
  * @param the priority of the timeout source. Typically this will be in the range between Glib::PRIORITY_DEFAULT and Glib::PRIORITY_HIGH.
  * @return A sigc::connection that can be used to disconnect the callback from the timeout source.
  */
-sigc::connection frame_source_add(const sigc::slot<bool>& callback, guint interval, gint priority = Glib::PRIORITY_DEFAULT);
+sigc::connection frame_source_add(const sigc::slot<bool>& callback, guint interval,
+                                  int priority = Glib::PRIORITY_DEFAULT);
 
-} //namespace Clutter
-
-#endif //_LIBCLUTTERMM_FRAME_SOURCE_H
+} // namespace Clutter
 
+#endif /* !_LIBCLUTTERMM_FRAME_SOURCE_H */
diff --git a/clutter/cluttermm/threads.cc b/clutter/cluttermm/threads.cc
index c79ca7c..ab1e140 100644
--- a/clutter/cluttermm/threads.cc
+++ b/clutter/cluttermm/threads.cc
@@ -36,12 +36,13 @@ public:
 
 private:
   sigc::slot_base slot_;
-  guint source_id_;
+  guint           source_id_;
 };
 
 SourceConnectionNode::SourceConnectionNode(const sigc::slot_base& slot)
 :
-  slot_(slot), source_id_(0)
+  slot_      (slot),
+  source_id_ (0)
 {
   slot_.set_parent(this, &SourceConnectionNode::notify);
 }
@@ -57,49 +58,49 @@ void* SourceConnectionNode::notify(void* data)
     // Removing the source triggers the destroy_notify_handler, wait until
     // that for deletion.
   }
+  return 0;
 }
 
 void SourceConnectionNode::destroy_notify_callback(void* data)
 {
   SourceConnectionNode* const self = static_cast<SourceConnectionNode*>(data);
-  if(self)
-  {
-    self->source_id_ = 0;
-    delete self;
-  }
+  self->source_id_ = 0;
+  delete self;
 }
 
+inline
 void SourceConnectionNode::install(guint source_id)
 {
   source_id_ = source_id;
 }
 
+inline
 sigc::slot_base* SourceConnectionNode::get_slot()
 {
   return &slot_;
 }
 
+static
 gboolean source_callback(void* data)
 {
   SourceConnectionNode* const conn_data = static_cast<SourceConnectionNode*>(data);
-
 #ifdef GLIBMM_EXCEPTIONS_ENABLED
   try
-  {
 #endif
+  {
     // Recreate the specific slot from the generic slot node
     return (*static_cast<sigc::slot<bool>*>(conn_data->get_slot()))();
-#ifdef GLIBMM_EXCEPTIONS_ENABLED
   }
+#ifdef GLIBMM_EXCEPTIONS_ENABLED
   catch(...)
   {
     Glib::exception_handlers_invoke();
   }
-#endif // GLIBMM_EXCEPTIONS_ENABLED
+#endif
   return FALSE;
 }
 
-}
+} // anonymous namespace
 
 namespace Clutter
 {
@@ -119,34 +120,40 @@ void threads_leave()
   clutter_threads_leave();
 }
 
+// TODO: This is unlikely to be thread-safe because of the returned connection
+// object.  It's the same problem as with the glibmm idle source.
 sigc::connection threads_add_idle(const sigc::slot<bool>& callback, int priority)
 {
   SourceConnectionNode* const conn_node = new SourceConnectionNode(callback);
-  const sigc::connection connection(*conn_node->get_slot());
+  const sigc::connection connection (*conn_node->get_slot());
 
-  guint id = clutter_threads_add_idle_full(priority, &source_callback, conn_node, &SourceConnectionNode::destroy_notify_callback);
-  conn_node->install(id);
+  conn_node->install(clutter_threads_add_idle_full(priority, &source_callback, conn_node,
+                                                   &SourceConnectionNode::destroy_notify_callback));
   return connection;
 }
 
-sigc::connection threads_add_timeout(const sigc::slot<bool>& callback, guint interval, gint priority)
+sigc::connection threads_add_timeout(const sigc::slot<bool>& callback,
+                                     guint interval, int priority)
 {
   SourceConnectionNode* const conn_node = new SourceConnectionNode(callback);
-  const sigc::connection connection(*conn_node->get_slot());
+  const sigc::connection connection (*conn_node->get_slot());
 
-  guint id = clutter_threads_add_timeout_full(priority, interval, &source_callback, conn_node, &SourceConnectionNode::destroy_notify_callback);
-  conn_node->install(id);
+  conn_node->install(clutter_threads_add_timeout_full(
+      priority, interval, &source_callback, conn_node,
+      &SourceConnectionNode::destroy_notify_callback));
   return connection;
 }
 
-sigc::connection threads_add_frame_source(const sigc::slot<bool>& callback, guint interval, gint priority)
+sigc::connection threads_add_frame_source(const sigc::slot<bool>& callback,
+                                          guint interval, int priority)
 {
   SourceConnectionNode* const conn_node = new SourceConnectionNode(callback);
-  const sigc::connection connection(*conn_node->get_slot());
+  const sigc::connection connection (*conn_node->get_slot());
 
-  guint id = clutter_threads_add_frame_source_full(priority, interval, &source_callback, conn_node, &SourceConnectionNode::destroy_notify_callback);
-  conn_node->install(id);
+  conn_node->install(clutter_threads_add_frame_source_full(
+      priority, interval, &source_callback, conn_node,
+      &SourceConnectionNode::destroy_notify_callback));
   return connection;
 }
 
-} //namespace Clutter
+} // namespace Clutter
diff --git a/clutter/src/timeline.ccg b/clutter/src/timeline.ccg
index 312d0a0..3cde058 100644
--- a/clutter/src/timeline.ccg
+++ b/clutter/src/timeline.ccg
@@ -20,13 +20,17 @@
 namespace Clutter
 {
 
+Timeline::Timeline(guint msecs)
+:
+  _CONSTRUCT("duration", msecs)
+{}
 
 Glib::StringArrayHandle Timeline::list_markers(gint frame_num) const
 {
-  gsize n_markers;
-  gchar** markers = clutter_timeline_list_markers(const_cast<ClutterTimeline*>(gobj()), frame_num, &n_markers);
+  gsize n_markers = 0;
+  gchar** const markers = clutter_timeline_list_markers(const_cast<ClutterTimeline*>(gobj()),
+                                                        frame_num, &n_markers);
   return Glib::StringArrayHandle(markers, n_markers, Glib::OWNERSHIP_DEEP);
 }
 
-} //namespace Clutter
-
+} // namespace Clutter
diff --git a/clutter/src/timeline.hg b/clutter/src/timeline.hg
index 4bee3b2..6d455f0 100644
--- a/clutter/src/timeline.hg
+++ b/clutter/src/timeline.hg
@@ -21,7 +21,6 @@
 _DEFS(cluttermm,clutter)
 _PINCLUDE(glibmm/private/object_p.h)
 
-
 namespace Clutter
 {
 
@@ -32,12 +31,11 @@ class Timeline : public Glib::Object
   _CLASS_GOBJECT(Timeline, ClutterTimeline, CLUTTER_TIMELINE, Glib::Object, GObject)
 
 protected:
-  _WRAP_CTOR(Timeline(guint num_frames, guint fps), clutter_timeline_new)
+  _WRAP_METHOD_DOCS_ONLY(clutter_timeline_new)
+  explicit Timeline(guint msecs);
+
 public:
-  _WRAP_CREATE(guint n_frames, guint fps)
-  
-  //We don't wrap this as a copy constructor because there is 
-  //no way to use copy constructors with RefPtr:
+  _WRAP_CREATE(guint msecs)
   _WRAP_METHOD(Glib::RefPtr<Timeline> clone() const, clutter_timeline_clone)
 
   _WRAP_METHOD(void set_duration(guint msecs), clutter_timeline_set_duration)
@@ -65,7 +63,7 @@ public:
   _WRAP_METHOD(TimelineDirection get_direction() const, clutter_timeline_get_direction)
 
   _WRAP_METHOD_DOCS_ONLY(clutter_timeline_list_markers)
-  Glib::StringArrayHandle list_markers(gint frame_num) const;
+  Glib::StringArrayHandle list_markers(int frame_num) const;
   _WRAP_METHOD(void add_marker_at_time(const Glib::ustring& marker_name, guint msecs), clutter_timeline_add_marker_at_time)
   _WRAP_METHOD(void add_marker_at_frame(const Glib::ustring& marker_name, guint frame_num), clutter_timeline_add_marker_at_frame)
   _WRAP_METHOD(bool has_marker(const Glib::ustring& marker_name), clutter_timeline_has_marker)
@@ -80,7 +78,7 @@ public:
   _WRAP_PROPERTY("num-frames", guint)
 
   _WRAP_SIGNAL(void completed(), "completed")
-  _WRAP_SIGNAL(void new_frame(gint fram_num), "new-frame")
+  _WRAP_SIGNAL(void new_frame(int fram_num), "new-frame")
   _WRAP_SIGNAL(void paused(), "paused")
   _WRAP_SIGNAL(void started(), "started")
 
@@ -92,4 +90,3 @@ public:
 };
 
 } // namespace Clutter
-
diff --git a/examples/test-actors.cc b/examples/test-actors.cc
index c503d22..1377cc1 100644
--- a/examples/test-actors.cc
+++ b/examples/test-actors.cc
@@ -113,7 +113,7 @@ int main(int argc, char *argv[])
 
   // Create a timeline to manage animation
   Glib::RefPtr<Clutter::Timeline> timeline =
-    Clutter::Timeline::create(360, 60); // num frames, fps
+    Clutter::Timeline::create(6000); // milliseconds
   timeline->set_loop(true); // have it loop
 
   // fire a callback for frame change
diff --git a/tests/test-alpha-creation.cc b/tests/test-alpha-creation.cc
index 93f1ce3..9aaf988 100644
--- a/tests/test-alpha-creation.cc
+++ b/tests/test-alpha-creation.cc
@@ -9,7 +9,7 @@ int main(int argc, char** argv)
 
   // Create a timeline to manage animation
   Glib::RefPtr<Clutter::Timeline> timeline =
-      Clutter::Timeline::create(360, 60); // num frames, fps
+      Clutter::Timeline::create(6000); // milliseconds
 
   // Set up some behaviours to handle scaling
   Glib::RefPtr<Clutter::Alpha> alpha =
diff --git a/tests/test-alpha-func.cc b/tests/test-alpha-func.cc
index 5900dbc..48ec435 100644
--- a/tests/test-alpha-func.cc
+++ b/tests/test-alpha-func.cc
@@ -6,7 +6,7 @@ Glib::RefPtr<Clutter::Actor> hand;
 
 guint32 alpha_val = 0;
 
-guint32 on_alpha(const Glib::RefPtr<Clutter::Alpha>& alpha)
+guint32 on_alpha(const Glib::RefPtr<Clutter::Alpha>&)
 {
   return ++alpha_val;
 }
@@ -17,11 +17,11 @@ main (int argc, char *argv[])
     // initialize the C++ wrapper types
     Clutter::init(&argc, &argv);
 
-    Glib::RefPtr<Clutter::Stage> stage = Clutter::Stage::get_default ();
+    Glib::RefPtr<Clutter::Stage> stage = Clutter::Stage::get_default();
 
     // Create a timeline to manage animation
     Glib::RefPtr<Clutter::Timeline> timeline =
-        Clutter::Timeline::create (1000, 60); // num frames, fps
+        Clutter::Timeline::create(16667); // milliseconds
     timeline->set_loop();
 
     // Set up some behaviours to handle scaling
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]