Make BusExpireList an opaque data type

2007-11-08  Havoc Pennington  <hp@redhat.com>

	* bus/connection.c, bus/expirelist.c: Make the BusExpireList
	struct opaque, adding accessors for manipulating the list. In this
	commit there should be no change in functionality or behavior. The
	purpose of this change is to improve encapsulation prior to fixing
	some bugs Kimmo Hämäläinen found where the timeout is not properly
	updated, since we need to e.g. take some action whenever adding
	and removing stuff from the expire list.

diff --git a/ChangeLog b/ChangeLog
index 96cecd01f76f651d34b4501ace27f3c050ccba4c..48fe581046593de95245e1ec2d60113955f96ff9 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,13 @@
+2008-03-04  Havoc Pennington  <hp@redhat.com>
+
+       * bus/connection.c, bus/expirelist.c: Make the BusExpireList
+       struct opaque, adding accessors for manipulating the list. In this
+       commit there should be no change in functionality or behavior. The
+       purpose of this change is to improve encapsulation prior to fixing
+       some bugs Kimmo Hämäläinen found where the timeout is not properly
+       updated, since we need to e.g. take some action whenever adding
+       and removing stuff from the expire list.
+
 2008-03-04  John (J5) Palmieri  <johnp@redhat.com>
 
        * fix broken poll on Mac OSX - build patch by Benjamin Reed

diff --git a/bus/connection.c b/bus/connection.c
index 512a2311eb7b4c5b715e758fca150d93535025a8..ed1b139165525a3c14de5beb5f843f282e94f6c6 100644 (file)
--- a/bus/connection.c
+++ b/bus/connection.c
@@ -1467,9 +1467,9 @@ bus_pending_reply_expired (BusExpireList *list,
       bus_transaction_cancel_and_free (transaction);
       return FALSE;
     }
-  
-  _dbus_list_remove_link (&connections->pending_replies->items,
-                          link);
+
+  bus_expire_list_remove_link (connections->pending_replies, link);
+
   bus_pending_reply_free (pending);
   bus_transaction_execute_and_free (transaction);
 
@@ -1488,14 +1488,14 @@ bus_connection_drop_pending_replies (BusConnections  *connections,
   _dbus_verbose ("Dropping pending replies that involve connection %p\n",
                  connection);
   
-  link = _dbus_list_get_first_link (&connections->pending_replies->items);
+  link = bus_expire_list_get_first_link (connections->pending_replies);
   while (link != NULL)
     {
       DBusList *next;
       BusPendingReply *pending;
 
-      next = _dbus_list_get_next_link (&connections->pending_replies->items,
-                                       link);
+      next = bus_expire_list_get_next_link (connections->pending_replies,
+                                            link);
       pending = link->data;
 
       if (pending->will_get_reply == connection)
@@ -1508,8 +1508,8 @@ bus_connection_drop_pending_replies (BusConnections  *connections,
                          pending->will_get_reply,
                          pending->reply_serial);
           
-          _dbus_list_remove_link (&connections->pending_replies->items,
-                                  link);
+          bus_expire_list_remove_link (connections->pending_replies,
+                                       link);
           bus_pending_reply_free (pending);
         }
       else if (pending->will_send_reply == connection)
@@ -1527,8 +1527,7 @@ bus_connection_drop_pending_replies (BusConnections  *connections,
           pending->expire_item.added_tv_sec = 0;
           pending->expire_item.added_tv_usec = 0;
 
-          bus_expire_timeout_set_interval (connections->pending_replies->timeout,
-                                           0);
+          bus_expire_list_recheck_immediately (connections->pending_replies);
         }
       
       link = next;
@@ -1549,8 +1548,8 @@ cancel_pending_reply (void *data)
 
   _dbus_verbose ("%s: d = %p\n", _DBUS_FUNCTION_NAME, d);
   
-  if (!_dbus_list_remove (&d->connections->pending_replies->items,
-                          d->pending))
+  if (!bus_expire_list_remove (d->connections->pending_replies,
+                               &d->pending->expire_item))
     _dbus_assert_not_reached ("pending reply did not exist to be cancelled");
 
   bus_pending_reply_free (d->pending); /* since it's been cancelled */
@@ -1597,7 +1596,7 @@ bus_connections_expect_reply (BusConnections  *connections,
   
   reply_serial = dbus_message_get_serial (reply_to_this);
 
-  link = _dbus_list_get_first_link (&connections->pending_replies->items);
+  link = bus_expire_list_get_first_link (connections->pending_replies);
   count = 0;
   while (link != NULL)
     {
@@ -1612,8 +1611,8 @@ bus_connections_expect_reply (BusConnections  *connections,
           return FALSE;
         }
       
-      link = _dbus_list_get_next_link (&connections->pending_replies->items,
-                                       link);
+      link = bus_expire_list_get_next_link (connections->pending_replies,
+                                            link);
       if (pending->will_get_reply == will_get_reply)
         ++count;
     }
@@ -1651,8 +1650,8 @@ bus_connections_expect_reply (BusConnections  *connections,
       return FALSE;
     }
   
-  if (!_dbus_list_prepend (&connections->pending_replies->items,
-                           pending))
+  if (!bus_expire_list_add (connections->pending_replies,
+                            &pending->expire_item))
     {
       BUS_SET_OOM (error);
       dbus_free (cprd);
@@ -1666,7 +1665,7 @@ bus_connections_expect_reply (BusConnections  *connections,
                                         cancel_pending_reply_data_free))
     {
       BUS_SET_OOM (error);
-      _dbus_list_remove (&connections->pending_replies->items, pending);
+      bus_expire_list_remove (connections->pending_replies, &pending->expire_item);
       dbus_free (cprd);
       bus_pending_reply_free (pending);
       return FALSE;
@@ -1699,9 +1698,9 @@ cancel_check_pending_reply (void *data)
   CheckPendingReplyData *d = data;
 
   _dbus_verbose ("%s: d = %p\n", _DBUS_FUNCTION_NAME, d);
-  
-  _dbus_list_prepend_link (&d->connections->pending_replies->items,
-                           d->link);
+
+  bus_expire_list_add_link (d->connections->pending_replies,
+                            d->link);
   d->link = NULL;
 }
 
@@ -1716,8 +1715,8 @@ check_pending_reply_data_free (void *data)
     {
       BusPendingReply *pending = d->link->data;
       
-      _dbus_assert (_dbus_list_find_last (&d->connections->pending_replies->items,
-                                          pending) == NULL);
+      _dbus_assert (!bus_expire_list_contains_item (d->connections->pending_replies,
+                                                    &pending->expire_item));
       
       bus_pending_reply_free (pending);
       _dbus_list_free_link (d->link);
@@ -1747,7 +1746,7 @@ bus_connections_check_reply (BusConnections *connections,
 
   reply_serial = dbus_message_get_reply_serial (reply);
 
-  link = _dbus_list_get_first_link (&connections->pending_replies->items);
+  link = bus_expire_list_get_first_link (connections->pending_replies);
   while (link != NULL)
     {
       BusPendingReply *pending = link->data;
@@ -1760,8 +1759,8 @@ bus_connections_check_reply (BusConnections *connections,
           break;
         }
       
-      link = _dbus_list_get_next_link (&connections->pending_replies->items,
-                                       link);
+      link = bus_expire_list_get_next_link (connections->pending_replies,
+                                            link);
     }
 
   if (link == NULL)
@@ -1791,11 +1790,10 @@ bus_connections_check_reply (BusConnections *connections,
   cprd->link = link;
   cprd->connections = connections;
   
-  _dbus_list_unlink (&connections->pending_replies->items,
-                     link);
+  bus_expire_list_unlink (connections->pending_replies,
+                          link);
   
-  _dbus_assert (_dbus_list_find_last (&connections->pending_replies->items,
-                                      link->data) == NULL);
+  _dbus_assert (!bus_expire_list_contains_item (connections->pending_replies, link->data));
 
   return TRUE;
 }

diff --git a/bus/expirelist.c b/bus/expirelist.c
index 6d5f4ba70110edbfdaf663feed930d0bbc26b1fc..ee7d0d56bbf08eb2894bd86e9dfc887631ffd871 100644 (file)
--- a/bus/expirelist.c
+++ b/bus/expirelist.c
@@ -27,6 +27,16 @@
 #include <dbus/dbus-mainloop.h>
 #include <dbus/dbus-timeout.h>
 
+struct BusExpireList
+{
+  DBusList      *items; /**< List of BusExpireItem */
+  DBusTimeout   *timeout;
+  DBusLoop      *loop;
+  BusExpireFunc  expire_func;
+  void          *data;
+  int            expire_after; /**< Expire after milliseconds (thousandths) */
+};
+
 static dbus_bool_t expire_timeout_handler (void *data);
 
 static void
@@ -92,8 +102,8 @@ bus_expire_list_free (BusExpireList *list)
 }
 
 void
-bus_expire_timeout_set_interval (DBusTimeout *timeout,
-                                 int          next_interval)
+bus_expire_timeout_set_interval (DBusTimeout   *timeout,
+                                 int            next_interval)
 {
   if (next_interval >= 0)
     {
@@ -101,17 +111,25 @@ bus_expire_timeout_set_interval (DBusTimeout *timeout,
                                   next_interval);
       _dbus_timeout_set_enabled (timeout, TRUE);
 
-      _dbus_verbose ("Enabled expire timeout with interval %d\n",
+      _dbus_verbose ("Enabled an expire timeout with interval %d\n",
                      next_interval);
     }
   else if (dbus_timeout_get_enabled (timeout))
     {
       _dbus_timeout_set_enabled (timeout, FALSE);
 
-      _dbus_verbose ("Disabled expire timeout\n");
+      _dbus_verbose ("Disabled an expire timeout\n");
     }
   else
-    _dbus_verbose ("No need to disable expire timeout\n");
+    _dbus_verbose ("No need to disable this expire timeout\n");
+}
+
+void
+bus_expire_list_recheck_immediately (BusExpireList *list)
+{
+  _dbus_verbose ("setting interval on expire list to 0 for immediate recheck\n");
+
+  bus_expire_timeout_set_interval (list->timeout, 0);
 }
 
 static int
@@ -201,6 +219,68 @@ expire_timeout_handler (void *data)
   return TRUE;
 }
 
+void
+bus_expire_list_remove_link (BusExpireList *list,
+                             DBusList      *link)
+{
+  _dbus_list_remove_link (&list->items,
+                          link);
+}
+
+dbus_bool_t
+bus_expire_list_remove (BusExpireList *list,
+                        BusExpireItem *item)
+{
+  return _dbus_list_remove (&list->items,
+                            item);
+}
+
+void
+bus_expire_list_unlink (BusExpireList *list,
+                        DBusList      *link)
+{
+  _dbus_list_unlink (&list->items, link);
+}
+
+dbus_bool_t
+bus_expire_list_add (BusExpireList *list,
+                     BusExpireItem *item)
+{
+  return _dbus_list_prepend (&list->items,
+                             item);
+}
+
+void
+bus_expire_list_add_link (BusExpireList *list,
+                          DBusList      *link)
+{
+  _dbus_assert (link->data != NULL);
+  
+  _dbus_list_prepend_link (&list->items,
+                           link);
+}
+
+DBusList*
+bus_expire_list_get_first_link (BusExpireList *list)
+{
+  return _dbus_list_get_first_link (&list->items);
+}
+
+DBusList*
+bus_expire_list_get_next_link (BusExpireList *list,
+                               DBusList      *link)
+{
+  return _dbus_list_get_next_link (&list->items,
+                                   link);
+}
+
+dbus_bool_t
+bus_expire_list_contains_item (BusExpireList *list,
+                               BusExpireItem *item)
+{
+  return _dbus_list_find_last (&list->items, item) != NULL;
+}
+
 #ifdef DBUS_BUILD_TESTS
 
 typedef struct
@@ -283,7 +363,7 @@ bus_expire_list_test (const DBusString *test_data_dir)
 
   item->item.added_tv_sec = tv_sec;
   item->item.added_tv_usec = tv_usec;
-  if (!_dbus_list_append (&list->items, item))
+  if (!bus_expire_list_add (list, &item->item))
     _dbus_assert_not_reached ("out of memory");
 
   next_interval =
@@ -307,7 +387,7 @@ bus_expire_list_test (const DBusString *test_data_dir)
   _dbus_verbose ("next_interval = %d\n", next_interval);
   _dbus_assert (next_interval == 1000 + EXPIRE_AFTER);
 
-  _dbus_list_clear (&list->items);
+  bus_expire_list_remove (list, &item->item);
   dbus_free (item);
   
   bus_expire_list_free (list);

diff --git a/bus/expirelist.h b/bus/expirelist.h
index 221b9ae9e73096bbb49d033f9f1480c6297fef2c..af0a0729107698609b0c4f54894cc31ee92a32b7 100644 (file)
--- a/bus/expirelist.h
+++ b/bus/expirelist.h
@@ -35,15 +35,6 @@ typedef dbus_bool_t (* BusExpireFunc) (BusExpireList *list,
                                        DBusList      *link,
                                        void          *data);
 
-struct BusExpireList
-{
-  DBusList      *items; /**< List of BusExpireItem */
-  DBusTimeout   *timeout;
-  DBusLoop      *loop;
-  BusExpireFunc  expire_func;
-  void          *data;
-  int            expire_after; /**< Expire after milliseconds (thousandths) */
-};
 
 /* embed this in a child expire item struct */
 struct BusExpireItem
@@ -52,18 +43,38 @@ struct BusExpireItem
   long added_tv_usec; /**< Time we were added (microsec component) */
 };
 
-BusExpireList* bus_expire_list_new    (DBusLoop      *loop,
-                                       int            expire_after,
-                                       BusExpireFunc  expire_func,
-                                       void          *data);
-void           bus_expire_list_free   (BusExpireList *list);
+BusExpireList* bus_expire_list_new                 (DBusLoop      *loop,
+                                                    int            expire_after,
+                                                    BusExpireFunc  expire_func,
+                                                    void          *data);
+void           bus_expire_list_free                (BusExpireList *list);
+void           bus_expire_list_recheck_immediately (BusExpireList *list);
+void           bus_expire_list_remove_link         (BusExpireList *list,
+                                                    DBusList      *link);
+dbus_bool_t    bus_expire_list_remove              (BusExpireList *list,
+                                                    BusExpireItem *item);
+DBusList*      bus_expire_list_get_first_link      (BusExpireList *list);
+DBusList*      bus_expire_list_get_next_link       (BusExpireList *list,
+                                                    DBusList      *link);
+dbus_bool_t    bus_expire_list_add                 (BusExpireList *list,
+                                                    BusExpireItem *item);
+void           bus_expire_list_add_link            (BusExpireList *list,
+                                                    DBusList      *link);
+dbus_bool_t    bus_expire_list_contains_item       (BusExpireList *list,
+                                                    BusExpireItem *item);
+void           bus_expire_list_unlink              (BusExpireList *list,
+                                                    DBusList      *link);
+
+/* this macro and function are semi-related utility functions, not really part of the
+ * BusExpireList API
+ */
 
 #define ELAPSED_MILLISECONDS_SINCE(orig_tv_sec, orig_tv_usec,   \
                                    now_tv_sec, now_tv_usec)     \
  (((double) (now_tv_sec) - (double) (orig_tv_sec)) * 1000.0 +   \
  ((double) (now_tv_usec) - (double) (orig_tv_usec)) / 1000.0)
 
-void bus_expire_timeout_set_interval (DBusTimeout *timeout,
-                                      int          next_interval);
+void bus_expire_timeout_set_interval (DBusTimeout   *timeout,
+                                      int            next_interval);
 
 #endif /* BUS_EXPIRE_LIST_H */