From f1ee877d76000920e6dbec1b59be1ffab39d2c81 Mon Sep 17 00:00:00 2001 From: Havoc Pennington Date: Sat, 12 Jul 2003 19:32:35 +0000 Subject: 2003-07-12 Havoc Pennington * dbus/dbus-object-registry.c: implement unit test, fix bugs discovered in process * dbus/dbus-connection.c: remove handler_table and register_handler(), add DBusObjectRegistry usage * dbus/dbus-objectid.c (dbus_object_id_is_null) (dbus_object_id_set_null): new functions --- dbus/dbus-connection.c | 262 +++++----------------- dbus/dbus-connection.h | 16 -- dbus/dbus-object-registry.c | 523 +++++++++++++++++++++++++++++++++++++++++--- dbus/dbus-object-registry.h | 21 +- dbus/dbus-object.h | 6 + dbus/dbus-objectid.c | 28 +++ dbus/dbus-objectid.h | 2 + 7 files changed, 589 insertions(+), 269 deletions(-) (limited to 'dbus') diff --git a/dbus/dbus-connection.c b/dbus/dbus-connection.c index ed29edc9..104fd41f 100644 --- a/dbus/dbus-connection.c +++ b/dbus/dbus-connection.c @@ -35,6 +35,7 @@ #include "dbus-threads.h" #include "dbus-protocol.h" #include "dbus-dataslot.h" +#include "dbus-object-registry.h" #if 0 #define CONNECTION_LOCK(connection) do { \ @@ -77,7 +78,7 @@ * you to set a function to be used to monitor the dispatch status. * * If you're using GLib or Qt add-on libraries for D-BUS, there are - * special convenience functions in those libraries that hide + * special convenience APIs in those libraries that hide * all the details of dispatch and watch/timeout monitoring. * For example, dbus_connection_setup_with_g_main(). * @@ -157,7 +158,6 @@ struct DBusConnection DBusWatchList *watches; /**< Stores active watches. */ DBusTimeoutList *timeouts; /**< Stores active timeouts. */ - DBusHashTable *handler_table; /**< Table of registered DBusMessageHandler */ DBusList *filter_list; /**< List of filters. */ DBusDataSlotList slot_list; /**< Data stored by allocated integer ID */ @@ -180,6 +180,7 @@ struct DBusConnection DBusList *link_cache; /**< A cache of linked list links to prevent contention * for the global linked list mempool lock */ + DBusObjectRegistry *objects; /**< Objects registered with this connection */ }; typedef struct @@ -664,7 +665,7 @@ _dbus_connection_new_for_transport (DBusTransport *transport) DBusConnection *connection; DBusWatchList *watch_list; DBusTimeoutList *timeout_list; - DBusHashTable *handler_table, *pending_replies; + DBusHashTable *pending_replies; DBusMutex *mutex; DBusCondVar *message_returned_cond; DBusCondVar *dispatch_cond; @@ -672,10 +673,10 @@ _dbus_connection_new_for_transport (DBusTransport *transport) DBusList *disconnect_link; DBusMessage *disconnect_message; DBusCounter *outgoing_counter; + DBusObjectRegistry *objects; watch_list = NULL; connection = NULL; - handler_table = NULL; pending_replies = NULL; timeout_list = NULL; mutex = NULL; @@ -685,6 +686,7 @@ _dbus_connection_new_for_transport (DBusTransport *transport) disconnect_link = NULL; disconnect_message = NULL; outgoing_counter = NULL; + objects = NULL; watch_list = _dbus_watch_list_new (); if (watch_list == NULL) @@ -692,13 +694,7 @@ _dbus_connection_new_for_transport (DBusTransport *transport) timeout_list = _dbus_timeout_list_new (); if (timeout_list == NULL) - goto error; - - handler_table = - _dbus_hash_table_new (DBUS_HASH_STRING, - dbus_free, NULL); - if (handler_table == NULL) - goto error; + goto error; pending_replies = _dbus_hash_table_new (DBUS_HASH_INT, @@ -737,6 +733,10 @@ _dbus_connection_new_for_transport (DBusTransport *transport) outgoing_counter = _dbus_counter_new (); if (outgoing_counter == NULL) goto error; + + objects = _dbus_object_registry_new (connection); + if (objects == NULL) + goto error; if (_dbus_modify_sigpipe) _dbus_disable_sigpipe (); @@ -749,7 +749,6 @@ _dbus_connection_new_for_transport (DBusTransport *transport) connection->transport = transport; connection->watches = watch_list; connection->timeouts = timeout_list; - connection->handler_table = handler_table; connection->pending_replies = pending_replies; connection->outgoing_counter = outgoing_counter; connection->filter_list = NULL; @@ -790,9 +789,6 @@ _dbus_connection_new_for_transport (DBusTransport *transport) if (connection != NULL) dbus_free (connection); - if (handler_table) - _dbus_hash_table_unref (handler_table); - if (pending_replies) _dbus_hash_table_unref (pending_replies); @@ -804,6 +800,9 @@ _dbus_connection_new_for_transport (DBusTransport *transport) if (outgoing_counter) _dbus_counter_unref (outgoing_counter); + + if (objects) + _dbus_object_registry_unref (objects); return NULL; } @@ -853,19 +852,9 @@ void _dbus_connection_handler_destroyed_locked (DBusConnection *connection, DBusMessageHandler *handler) { - DBusHashIter iter; DBusList *link; CONNECTION_LOCK (connection); - - _dbus_hash_iter_init (connection->handler_table, &iter); - while (_dbus_hash_iter_next (&iter)) - { - DBusMessageHandler *h = _dbus_hash_iter_get_value (&iter); - - if (h == handler) - _dbus_hash_iter_remove_entry (&iter); - } link = _dbus_list_get_first_link (&connection->filter_list); while (link != NULL) @@ -1035,7 +1024,6 @@ free_outgoing_message (void *element, static void _dbus_connection_last_unref (DBusConnection *connection) { - DBusHashIter iter; DBusList *link; _dbus_verbose ("Finalizing connection %p\n", connection); @@ -1048,6 +1036,8 @@ _dbus_connection_last_unref (DBusConnection *connection) _dbus_assert (!_dbus_transport_get_is_connected (connection->transport)); /* ---- We're going to call various application callbacks here, hope it doesn't break anything... */ + _dbus_object_registry_free_all_unlocked (connection->objects); + dbus_connection_set_dispatch_status_function (connection, NULL, NULL, NULL); dbus_connection_set_wakeup_main_function (connection, NULL, NULL, NULL); dbus_connection_set_unix_user_function (connection, NULL, NULL, NULL); @@ -1061,14 +1051,6 @@ _dbus_connection_last_unref (DBusConnection *connection) _dbus_data_slot_list_free (&connection->slot_list); /* ---- Done with stuff that invokes application callbacks */ - _dbus_hash_iter_init (connection->handler_table, &iter); - while (_dbus_hash_iter_next (&iter)) - { - DBusMessageHandler *h = _dbus_hash_iter_get_value (&iter); - - _dbus_message_handler_remove_connection (h, connection); - } - link = _dbus_list_get_first_link (&connection->filter_list); while (link != NULL) { @@ -1080,8 +1062,7 @@ _dbus_connection_last_unref (DBusConnection *connection) link = next; } - _dbus_hash_table_unref (connection->handler_table); - connection->handler_table = NULL; + _dbus_object_registry_unref (connection->objects); _dbus_hash_table_unref (connection->pending_replies); connection->pending_replies = NULL; @@ -2237,12 +2218,10 @@ dbus_connection_get_dispatch_status (DBusConnection *connection) DBusDispatchStatus dbus_connection_dispatch (DBusConnection *connection) { - DBusMessageHandler *handler; DBusMessage *message; DBusList *link, *filter_list_copy, *message_link; DBusHandlerResult result; ReplyHandlerData *reply_handler_data; - const char *name; dbus_int32_t reply_serial; DBusDispatchStatus status; @@ -2373,30 +2352,19 @@ dbus_connection_dispatch (DBusConnection *connection) CONNECTION_LOCK (connection); goto out; } - - name = dbus_message_get_name (message); - if (name != NULL) - { - handler = _dbus_hash_table_lookup_string (connection->handler_table, - name); - if (handler != NULL) - { - /* We're still protected from dispatch() reentrancy here - * since we acquired the dispatcher - */ - CONNECTION_UNLOCK (connection); - - _dbus_verbose (" running app handler on message %p (%s)\n", - message, dbus_message_get_name (message)); - - result = _dbus_message_handler_handle_message (handler, connection, - message); - CONNECTION_LOCK (connection); - if (result == DBUS_HANDLER_RESULT_REMOVE_MESSAGE) - goto out; - } - } + /* We're still protected from dispatch() reentrancy here + * since we acquired the dispatcher + */ + _dbus_verbose (" running object handler on message %p (%s)\n", + message, dbus_message_get_name (message)); + + result = _dbus_object_registry_handle_and_unlock (connection->objects, + message); + CONNECTION_LOCK (connection); + if (result == DBUS_HANDLER_RESULT_REMOVE_MESSAGE) + goto out; + _dbus_verbose (" done dispatching %p (%s) on connection %p\n", message, dbus_message_get_name (message), connection); @@ -2721,8 +2689,8 @@ dbus_connection_set_unix_user_function (DBusConnection *connection, /** * Adds a message filter. Filters are handlers that are run on - * all incoming messages, prior to the normal handlers - * registered with dbus_connection_register_handler(). + * all incoming messages, prior to the objects + * registered with dbus_connection_register_object(). * Filters are run in the order that they were added. * The same handler can be added as a filter more than once, in * which case it will be run more than once. @@ -2795,153 +2763,6 @@ dbus_connection_remove_filter (DBusConnection *connection, CONNECTION_UNLOCK (connection); } -/** - * Registers a handler for a list of message names. A single handler - * can be registered for any number of message names, but each message - * name can only have one handler at a time. It's not allowed to call - * this function with the name of a message that already has a - * handler. If the function returns #FALSE, the handlers were not - * registered due to lack of memory. - * - * The connection does NOT add a reference to the message handler; - * instead, if the message handler is finalized, the connection simply - * forgets about it. Thus the caller of this function must keep a - * reference to the message handler. - * - * @todo the messages_to_handle arg may be more convenient if it's a - * single string instead of an array. Though right now MessageHandler - * is sort of designed to say be associated with an entire object with - * multiple methods, that's why for example the connection only - * weakrefs it. So maybe the "manual" API should be different. - * - * @param connection the connection - * @param handler the handler - * @param messages_to_handle the messages to handle - * @param n_messages the number of message names in messages_to_handle - * @returns #TRUE on success, #FALSE if no memory or another handler already exists - * - **/ -dbus_bool_t -dbus_connection_register_handler (DBusConnection *connection, - DBusMessageHandler *handler, - const char **messages_to_handle, - int n_messages) -{ - int i; - - _dbus_return_val_if_fail (connection != NULL, FALSE); - _dbus_return_val_if_fail (handler != NULL, FALSE); - _dbus_return_val_if_fail (n_messages >= 0, FALSE); - _dbus_return_val_if_fail (n_messages == 0 || messages_to_handle != NULL, FALSE); - - CONNECTION_LOCK (connection); - i = 0; - while (i < n_messages) - { - DBusHashIter iter; - char *key; - - key = _dbus_strdup (messages_to_handle[i]); - if (key == NULL) - goto failed; - - if (!_dbus_hash_iter_lookup (connection->handler_table, - key, TRUE, - &iter)) - { - dbus_free (key); - goto failed; - } - - if (_dbus_hash_iter_get_value (&iter) != NULL) - { - _dbus_warn ("Bug in application: attempted to register a second handler for %s\n", - messages_to_handle[i]); - dbus_free (key); /* won't have replaced the old key with the new one */ - goto failed; - } - - if (!_dbus_message_handler_add_connection (handler, connection)) - { - _dbus_hash_iter_remove_entry (&iter); - /* key has freed on nuking the entry */ - goto failed; - } - - _dbus_hash_iter_set_value (&iter, handler); - - ++i; - } - - CONNECTION_UNLOCK (connection); - return TRUE; - - failed: - /* unregister everything registered so far, - * so we don't fail partially - */ - dbus_connection_unregister_handler (connection, - handler, - messages_to_handle, - i); - - CONNECTION_UNLOCK (connection); - return FALSE; -} - -/** - * Unregisters a handler for a list of message names. The handlers - * must have been previously registered. - * - * @param connection the connection - * @param handler the handler - * @param messages_to_handle the messages to handle - * @param n_messages the number of message names in messages_to_handle - * - **/ -void -dbus_connection_unregister_handler (DBusConnection *connection, - DBusMessageHandler *handler, - const char **messages_to_handle, - int n_messages) -{ - int i; - - _dbus_return_if_fail (connection != NULL); - _dbus_return_if_fail (handler != NULL); - _dbus_return_if_fail (n_messages >= 0); - _dbus_return_if_fail (n_messages == 0 || messages_to_handle != NULL); - - CONNECTION_LOCK (connection); - i = 0; - while (i < n_messages) - { - DBusHashIter iter; - - if (!_dbus_hash_iter_lookup (connection->handler_table, - (char*) messages_to_handle[i], FALSE, - &iter)) - { - _dbus_warn ("Bug in application: attempted to unregister handler for %s which was not registered\n", - messages_to_handle[i]); - } - else if (_dbus_hash_iter_get_value (&iter) != handler) - { - _dbus_warn ("Bug in application: attempted to unregister handler for %s which was registered by a different handler\n", - messages_to_handle[i]); - } - else - { - _dbus_hash_iter_remove_entry (&iter); - _dbus_message_handler_remove_connection (handler, connection); - } - - ++i; - } - - CONNECTION_UNLOCK (connection); -} - /** * Registers an object with the connection. This object is assigned an * object ID, and will be visible under this ID and with the provided @@ -2951,7 +2772,11 @@ dbus_connection_unregister_handler (DBusConnection *connection, * * As a side effect of calling this function, the "registered" * callback in the #DBusObjectVTable will be invoked. - * + * + * If the object is deleted, be sure to unregister it with + * dbus_connection_unregister_object() or it will continue to get + * messages. + * * @param connection the connection to register the instance with * @param interfaces #NULL-terminated array of interface names the instance supports * @param vtable virtual table of functions for manipulating the instance @@ -2966,8 +2791,15 @@ dbus_connection_register_object (DBusConnection *connection, void *object_impl, DBusObjectID *object_id) { + _dbus_return_val_if_fail (connection != NULL, FALSE); + + CONNECTION_LOCK (connection); - return FALSE; + return _dbus_object_registry_add_and_unlock (connection->objects, + interfaces, + vtable, + object_impl, + object_id); } /** @@ -2983,8 +2815,12 @@ void dbus_connection_unregister_object (DBusConnection *connection, const DBusObjectID *object_id) { - + _dbus_return_if_fail (connection != NULL); + + CONNECTION_LOCK (connection); + return _dbus_object_registry_remove_and_unlock (connection->objects, + object_id); } static DBusDataSlotAllocator slot_allocator; diff --git a/dbus/dbus-connection.h b/dbus/dbus-connection.h index 6c0da920..7bf1221a 100644 --- a/dbus/dbus-connection.h +++ b/dbus/dbus-connection.h @@ -36,15 +36,8 @@ DBUS_BEGIN_DECLS; typedef struct DBusWatch DBusWatch; typedef struct DBusTimeout DBusTimeout; typedef struct DBusMessageHandler DBusMessageHandler; -typedef struct DBusObject DBusObject; typedef struct DBusPreallocatedSend DBusPreallocatedSend; -typedef enum -{ - DBUS_HANDLER_RESULT_REMOVE_MESSAGE, /**< Remove this message, no further processing. */ - DBUS_HANDLER_RESULT_ALLOW_MORE_HANDLERS /**< Run any additional handlers that are interested in this message. */ -} DBusHandlerResult; - typedef enum { DBUS_WATCH_READABLE = 1 << 0, /**< As in POLLIN */ @@ -162,15 +155,6 @@ dbus_bool_t dbus_connection_add_filter (DBusConnection *connection, void dbus_connection_remove_filter (DBusConnection *connection, DBusMessageHandler *handler); -dbus_bool_t dbus_connection_register_handler (DBusConnection *connection, - DBusMessageHandler *handler, - const char **messages_to_handle, - int n_messages); -void dbus_connection_unregister_handler (DBusConnection *connection, - DBusMessageHandler *handler, - const char **messages_to_handle, - int n_messages); - /* Objects */ dbus_bool_t dbus_connection_register_object (DBusConnection *connection, const char **interfaces, diff --git a/dbus/dbus-object-registry.c b/dbus/dbus-object-registry.c index eba2d8fb..64320179 100644 --- a/dbus/dbus-object-registry.c +++ b/dbus/dbus-object-registry.c @@ -23,6 +23,7 @@ #include "dbus-object-registry.h" #include "dbus-connection-internal.h" #include "dbus-internals.h" +#include "dbus-hash.h" #include /** @@ -30,16 +31,27 @@ * @ingroup DBusInternals * @brief DBusObjectRegistry is used by DBusConnection to track object IDs * - * Types and functions related to DBusObjectRegistry + * Types and functions related to DBusObjectRegistry. These + * are all internal. * * @{ */ typedef struct DBusObjectEntry DBusObjectEntry; +typedef struct DBusInterfaceEntry DBusInterfaceEntry; + +#define DBUS_MAX_OBJECTS_PER_INTERFACE 65535 +struct DBusInterfaceEntry +{ + unsigned int n_objects : 16; /**< Number of objects with this interface */ + unsigned int n_allocated : 16; /**< Allocated size of objects array */ + dbus_uint16_t *objects; /**< Index of each object with the interface */ + char name[4]; /**< Name of interface (actually allocated larger) */ +}; /* 14 bits for object index, 32K objects */ #define DBUS_OBJECT_INDEX_BITS (14) -#define DBUS_OBJECT_INDEX_MASK (0x7fff) +#define DBUS_OBJECT_INDEX_MASK (0x3fff) #define DBUS_MAX_OBJECTS_PER_CONNECTION DBUS_OBJECT_INDEX_MASK struct DBusObjectEntry { @@ -48,6 +60,7 @@ struct DBusObjectEntry void *object_impl; /**< Pointer to application-supplied implementation */ const DBusObjectVTable *vtable; /**< Virtual table for this object */ + DBusInterfaceEntry **interfaces; /**< NULL-terminated list of interfaces */ }; struct DBusObjectRegistry @@ -58,21 +71,58 @@ struct DBusObjectRegistry DBusObjectEntry *entries; int n_entries_allocated; int n_entries_used; + + DBusHashTable *interface_table; }; +static void +free_interface_entry (void *entry) +{ + DBusInterfaceEntry *iface = entry; + + if (iface == NULL) /* DBusHashTable stupidity */ + return; + + dbus_free (iface->objects); + dbus_free (iface); +} + DBusObjectRegistry* _dbus_object_registry_new (DBusConnection *connection) { DBusObjectRegistry *registry; + DBusHashTable *interface_table; + + /* the connection passed in here isn't fully constructed, + * so don't do anything more than store a pointer to + * it + */ + registry = NULL; + interface_table = NULL; + registry = dbus_new0 (DBusObjectRegistry, 1); if (registry == NULL) - return NULL; + goto oom; + + interface_table = _dbus_hash_table_new (DBUS_HASH_STRING, + NULL, free_interface_entry); + if (interface_table == NULL) + goto oom; registry->refcount = 1; registry->connection = connection; - + registry->interface_table = interface_table; + return registry; + + oom: + if (registry) + dbus_free (registry); + if (interface_table) + _dbus_hash_table_unref (interface_table); + + return NULL; } void @@ -92,8 +142,20 @@ _dbus_object_registry_unref (DBusObjectRegistry *registry) if (registry->refcount == 0) { + int i; + _dbus_assert (registry->n_entries_used == 0); + _dbus_assert (_dbus_hash_table_get_n_entries (registry->interface_table) == 0); + i = 0; + while (i < registry->n_entries_allocated) + { + if (registry->entries[i].interfaces) + dbus_free (registry->entries[i].interfaces); + ++i; + } + + _dbus_hash_table_unref (registry->interface_table); dbus_free (registry->entries); dbus_free (registry); } @@ -104,7 +166,7 @@ _dbus_object_registry_unref (DBusObjectRegistry *registry) (((dbus_uint32_t)(entry)->id_times_used) << DBUS_OBJECT_INDEX_BITS)) #define ID_TO_INDEX(id) \ - (((dbus_uint32_t) (id)) | DBUS_OBJECT_INDEX_MASK) + (((dbus_uint32_t) (id)) & DBUS_OBJECT_INDEX_MASK) #define ID_TO_TIMES_USED(id) \ (((dbus_uint32_t) (id)) >> DBUS_OBJECT_INDEX_BITS) @@ -121,7 +183,7 @@ validate_id (DBusObjectRegistry *registry, idx = ID_TO_INDEX (low_bits); times_used = ID_TO_TIMES_USED (low_bits); - + if (idx >= registry->n_entries_allocated) return NULL; if (registry->entries[idx].vtable == NULL) @@ -141,12 +203,154 @@ info_from_entry (DBusObjectRegistry *registry, { info->connection = registry->connection; info->object_impl = entry->object_impl; - dbus_object_id_set_high_bits (&info->object_id, - _dbus_connection_get_id (registry->connection)); +#ifdef DBUS_BUILD_TESTS + if (registry->connection) +#endif + dbus_object_id_set_high_bits (&info->object_id, + _dbus_connection_get_id (registry->connection)); +#ifdef DBUS_BUILD_TESTS + else + dbus_object_id_set_high_bits (&info->object_id, 1); +#endif + dbus_object_id_set_low_bits (&info->object_id, ENTRY_TO_ID (entry)); } +static DBusInterfaceEntry* +lookup_interface (DBusObjectRegistry *registry, + const char *name, + dbus_bool_t create_if_not_found) +{ + DBusInterfaceEntry *entry; + int sz; + int len; + + entry = _dbus_hash_table_lookup_string (registry->interface_table, + name); + if (entry != NULL || !create_if_not_found) + return entry; + + _dbus_assert (create_if_not_found); + + len = strlen (name); + sz = _DBUS_STRUCT_OFFSET (DBusInterfaceEntry, name) + len + 1; + entry = dbus_malloc (sz); + if (entry == NULL) + return NULL; + entry->n_objects = 0; + entry->n_allocated = 0; + entry->objects = NULL; + memcpy (entry->name, name, len + 1); + + if (!_dbus_hash_table_insert_string (registry->interface_table, + entry->name, entry)) + { + dbus_free (entry); + return NULL; + } + + return entry; +} + +static void +delete_interface (DBusObjectRegistry *registry, + DBusInterfaceEntry *entry) +{ + _dbus_hash_table_remove_string (registry->interface_table, + entry->name); +} + +static dbus_bool_t +interface_entry_add_object (DBusInterfaceEntry *entry, + dbus_uint16_t object_index) +{ + if (entry->n_objects == entry->n_allocated) + { + unsigned int new_alloc; + dbus_uint16_t *new_objects; + + if (entry->n_allocated == 0) + new_alloc = 2; + else + new_alloc = entry->n_allocated * 2; + + /* Right now MAX_OBJECTS_PER_INTERFACE can't possibly be reached + * since the max number of objects _total_ is smaller, but the + * code is here for future robustness. + */ + + if (new_alloc > DBUS_MAX_OBJECTS_PER_INTERFACE) + new_alloc = DBUS_MAX_OBJECTS_PER_INTERFACE; + if (new_alloc == entry->n_allocated) + { + _dbus_warn ("Attempting to register another instance with interface %s, but max count %d reached\n", + entry->name, DBUS_MAX_OBJECTS_PER_INTERFACE); + return FALSE; + } + + new_objects = dbus_realloc (entry->objects, new_alloc * sizeof (dbus_uint16_t)); + if (new_objects == NULL) + return FALSE; + entry->objects = new_objects; + entry->n_allocated = new_alloc; + } + + _dbus_assert (entry->n_objects < entry->n_allocated); + + entry->objects[entry->n_objects] = object_index; + entry->n_objects += 1; + + return TRUE; +} + +static void +interface_entry_remove_object (DBusInterfaceEntry *entry, + dbus_uint16_t object_index) +{ + unsigned int i; + + i = 0; + while (i < entry->n_objects) + { + if (entry->objects[i] == object_index) + break; + ++i; + } + + if (i == entry->n_objects) + { + _dbus_assert_not_reached ("Tried to remove object from an interface that didn't list that object\n"); + return; + } + + memmove (&entry->objects[i], + &entry->objects[i+1], + (entry->n_objects - i - 1) * sizeof (entry->objects[0])); + entry->n_objects -= 1; +} + +static void +object_remove_from_interfaces (DBusObjectRegistry *registry, + DBusObjectEntry *entry) +{ + if (entry->interfaces != NULL) + { + int i; + + i = 0; + while (entry->interfaces[i] != NULL) + { + DBusInterfaceEntry *iface = entry->interfaces[i]; + + interface_entry_remove_object (iface, entry->id_index); + if (iface->n_objects == 0) + delete_interface (registry, iface); + ++i; + } + } +} + dbus_bool_t _dbus_object_registry_add_and_unlock (DBusObjectRegistry *registry, const char **interfaces, @@ -154,9 +358,10 @@ _dbus_object_registry_add_and_unlock (DBusObjectRegistry *registry, void *object_impl, DBusObjectID *object_id) { + int idx; int i; DBusObjectInfo info; - + if (registry->n_entries_used == registry->n_entries_allocated) { DBusObjectEntry *new_entries; @@ -170,7 +375,7 @@ _dbus_object_registry_add_and_unlock (DBusObjectRegistry *registry, { _dbus_warn ("Attempting to register a new D-BUS object, but maximum object count of %d reached\n", DBUS_MAX_OBJECTS_PER_CONNECTION); - return FALSE; + goto out_0; } new_alloc = registry->n_entries_allocated * 2; @@ -182,7 +387,7 @@ _dbus_object_registry_add_and_unlock (DBusObjectRegistry *registry, new_alloc * sizeof (DBusObjectEntry)); if (new_entries == NULL) - return FALSE; + goto out_0; memset (&new_entries[registry->n_entries_allocated], '\0', @@ -199,7 +404,7 @@ _dbus_object_registry_add_and_unlock (DBusObjectRegistry *registry, */ if (registry->entries[registry->n_entries_used].vtable == NULL) { - i = registry->n_entries_used; + idx = registry->n_entries_used; } else { @@ -213,34 +418,125 @@ _dbus_object_registry_add_and_unlock (DBusObjectRegistry *registry, * the range [0, n_entries_used). Thus, there is * at least one blank entry inside that range. */ - i = 0; - while (i < registry->n_entries_used) + idx = 0; + while (idx < registry->n_entries_used) { - if (registry->entries[i].vtable == NULL) + if (registry->entries[idx].vtable == NULL) break; - ++i; + ++idx; } - _dbus_assert (i < registry->n_entries_used); + _dbus_assert (idx < registry->n_entries_used); } + + registry->entries[idx].id_index = idx; + /* Overflow is OK here, but zero isn't as it's a null ID */ + registry->entries[idx].id_times_used += 1; + if (registry->entries[idx].id_times_used == 0) + registry->entries[idx].id_times_used += 1; + + registry->entries[idx].vtable = vtable; + registry->entries[idx].object_impl = object_impl; - registry->entries[i].id_index = i; - /* Overflow is OK here */ - registry->entries[i].id_times_used += 1; + registry->n_entries_used += 1; - registry->entries[i].vtable = vtable; - registry->entries[i].object_impl = object_impl; + i = 0; + if (interfaces != NULL) + { + while (interfaces[i] != NULL) + ++i; + } + + if (i > 0) + { + DBusInterfaceEntry **new_interfaces; + + new_interfaces = + dbus_realloc (registry->entries[idx].interfaces, + (i + 1) * sizeof (DBusInterfaceEntry*)); + + if (new_interfaces == NULL) + { + /* maintain invariant that .interfaces array points to something + * valid in oom handler (entering this function it pointed to + * stale data but a valid malloc block) + */ + dbus_free (registry->entries[idx].interfaces); + registry->entries[idx].interfaces = NULL; + goto out_1; + } - info_from_entry (registry, &info, ®istry->entries[i]); + /* NULL-init so it's NULL-terminated and the OOM + * case can see how far we got + */ + while (i >= 0) + { + new_interfaces[i] = NULL; + --i; + } + + registry->entries[idx].interfaces = new_interfaces; + } + else + { + dbus_free (registry->entries[idx].interfaces); + registry->entries[idx].interfaces = NULL; + } + + /* Fill in interfaces */ + if (interfaces != NULL) + { + i = 0; + while (interfaces[i] != NULL) + { + DBusInterfaceEntry *iface; + + iface = lookup_interface (registry, interfaces[i], + TRUE); + if (iface == NULL) + goto out_1; + + if (!interface_entry_add_object (iface, idx)) + { + if (iface->n_objects == 0) + delete_interface (registry, iface); + goto out_1; + } + + registry->entries[idx].interfaces[i] = iface; + + ++i; + } + } + + info_from_entry (registry, &info, ®istry->entries[idx]); if (object_id) *object_id = info.object_id; /* Drop lock and invoke application code */ - _dbus_connection_unlock (registry->connection); - +#ifdef DBUS_BUILD_TESTS + if (registry->connection) +#endif + _dbus_connection_unlock (registry->connection); + (* vtable->registered) (&info); return TRUE; + + out_1: + registry->entries[idx].vtable = NULL; + registry->entries[idx].object_impl = NULL; + registry->n_entries_used -= 1; + + object_remove_from_interfaces (registry, + ®istry->entries[idx]); + + out_0: +#ifdef DBUS_BUILD_TESTS + if (registry->connection) +#endif + _dbus_connection_unlock (registry->connection); + return FALSE; } void @@ -255,33 +551,44 @@ _dbus_object_registry_remove_and_unlock (DBusObjectRegistry *registry, if (entry == NULL) { _dbus_warn ("Tried to unregister a nonexistent D-BUS object ID\n"); +#ifdef DBUS_BUILD_TESTS + if (registry->connection) +#endif + _dbus_connection_unlock (registry->connection); + return; } + object_remove_from_interfaces (registry, entry); + info_from_entry (registry, &info, entry); vtable = entry->vtable; entry->vtable = NULL; entry->object_impl = NULL; registry->n_entries_used -= 1; - + /* Drop lock and invoke application code */ - _dbus_connection_unlock (registry->connection); +#ifdef DBUS_BUILD_TESTS + if (registry->connection) +#endif + _dbus_connection_unlock (registry->connection); (* vtable->unregistered) (&info); } -void +DBusHandlerResult _dbus_object_registry_handle_and_unlock (DBusObjectRegistry *registry, DBusMessage *message) { /* FIXME */ + return DBUS_HANDLER_RESULT_ALLOW_MORE_HANDLERS; } void _dbus_object_registry_free_all_unlocked (DBusObjectRegistry *registry) { int i; - + i = 0; while (registry->n_entries_used > 0) { @@ -291,6 +598,9 @@ _dbus_object_registry_free_all_unlocked (DBusObjectRegistry *registry) DBusObjectInfo info; const DBusObjectVTable *vtable; + object_remove_from_interfaces (registry, + ®istry->entries[i]); + info_from_entry (registry, &info, ®istry->entries[i]); vtable = registry->entries[i].vtable; registry->entries[i].vtable = NULL; @@ -313,6 +623,157 @@ _dbus_object_registry_free_all_unlocked (DBusObjectRegistry *registry) #include "dbus-test.h" #include +static void +noop_message_function (DBusObjectInfo *info, + DBusMessage *message) +{ + /* nothing */ +} + +static void +add_and_remove_objects (DBusObjectRegistry *registry) +{ +#define N_OBJECTS 73 + DBusObjectID ids[N_OBJECTS]; + const char *zero_interfaces[] = { NULL }; + const char *one_interface[] = { "org.freedesktop.Test.Blah", NULL }; + const char *three_interfaces[] = { "org.freedesktop.Test.Blah", + "org.freedesktop.Test.Baz", + "org.freedesktop.Test.Foo", + NULL }; + int i; + + i = 0; + while (i < N_OBJECTS) + { + DBusCallbackObject *callback; + const char **interfaces; + + callback = dbus_callback_object_new (noop_message_function, NULL, NULL); + if (callback == NULL) + goto out; + + switch (i % 3) + { + case 0: + interfaces = zero_interfaces; + break; + case 1: + interfaces = one_interface; + break; + case 2: + interfaces = three_interfaces; + break; + } + + if (!_dbus_object_registry_add_and_unlock (registry, + interfaces, + dbus_callback_object_vtable, + callback, + &ids[i])) + { + dbus_callback_object_unref (callback); + goto out; + } + + dbus_callback_object_unref (callback); + + ++i; + } + + i = 0; + while (i < N_OBJECTS) + { + if (i > (N_OBJECTS - 20) || (i % 3) == 0) + { + _dbus_object_registry_remove_and_unlock (registry, + &ids[i]); + dbus_object_id_set_null (&ids[i]); + } + + ++i; + } + + i = 0; + while (i < N_OBJECTS) + { + if (dbus_object_id_is_null (&ids[i])) + { + DBusCallbackObject *callback; + const char **interfaces; + + callback = dbus_callback_object_new (noop_message_function, NULL, NULL); + if (callback == NULL) + goto out; + + switch (i % 4) + { + case 0: + interfaces = NULL; + break; + case 1: + interfaces = zero_interfaces; + break; + case 2: + interfaces = one_interface; + break; + case 3: + interfaces = three_interfaces; + break; + } + + if (!_dbus_object_registry_add_and_unlock (registry, + interfaces, + dbus_callback_object_vtable, + callback, + &ids[i])) + { + dbus_callback_object_unref (callback); + goto out; + } + + dbus_callback_object_unref (callback); + } + + ++i; + } + + i = 0; + while (i < (N_OBJECTS - 30)) + { + _dbus_assert (!dbus_object_id_is_null (&ids[i])); + + _dbus_object_registry_remove_and_unlock (registry, + &ids[i]); + ++i; + } + + out: + /* unregister the rest this way, to test this function */ + _dbus_object_registry_free_all_unlocked (registry); +} + +static dbus_bool_t +object_registry_test_iteration (void *data) +{ + DBusObjectRegistry *registry; + + registry = _dbus_object_registry_new (NULL); + if (registry == NULL) + return TRUE; + + /* we do this twice since realloc behavior will differ each time, + * and the IDs will get recycled leading to slightly different + * codepaths + */ + add_and_remove_objects (registry); + add_and_remove_objects (registry); + + _dbus_object_registry_unref (registry); + + return TRUE; +} + /** * @ingroup DBusObjectRegistry * Unit test for DBusObjectRegistry @@ -321,7 +782,9 @@ _dbus_object_registry_free_all_unlocked (DBusObjectRegistry *registry) dbus_bool_t _dbus_object_registry_test (void) { - /* FIXME */ + _dbus_test_oom_handling ("object registry", + object_registry_test_iteration, + NULL); return TRUE; } diff --git a/dbus/dbus-object-registry.h b/dbus/dbus-object-registry.h index d33664e5..57009c87 100644 --- a/dbus/dbus-object-registry.h +++ b/dbus/dbus-object-registry.h @@ -33,16 +33,17 @@ DBusObjectRegistry* _dbus_object_registry_new (DBusConnection *connection) void _dbus_object_registry_ref (DBusObjectRegistry *registry); void _dbus_object_registry_unref (DBusObjectRegistry *registry); -dbus_bool_t _dbus_object_registry_add_and_unlock (DBusObjectRegistry *registry, - const char **interfaces, - const DBusObjectVTable *vtable, - void *object_impl, - DBusObjectID *object_id); -void _dbus_object_registry_remove_and_unlock (DBusObjectRegistry *registry, - const DBusObjectID *object_id); -void _dbus_object_registry_handle_and_unlock (DBusObjectRegistry *registry, - DBusMessage *message); -void _dbus_object_registry_free_all_unlocked (DBusObjectRegistry *registry); +dbus_bool_t _dbus_object_registry_add_and_unlock (DBusObjectRegistry *registry, + const char **interfaces, + const DBusObjectVTable *vtable, + void *object_impl, + DBusObjectID *object_id); +void _dbus_object_registry_remove_and_unlock (DBusObjectRegistry *registry, + const DBusObjectID *object_id); +DBusHandlerResult _dbus_object_registry_handle_and_unlock (DBusObjectRegistry *registry, + DBusMessage *message); +void _dbus_object_registry_free_all_unlocked (DBusObjectRegistry *registry); + DBUS_END_DECLS; diff --git a/dbus/dbus-object.h b/dbus/dbus-object.h index b05d9c4b..84fb2ede 100644 --- a/dbus/dbus-object.h +++ b/dbus/dbus-object.h @@ -39,6 +39,12 @@ typedef struct DBusObjectVTable DBusObjectVTable; typedef struct DBusObjectInfo DBusObjectInfo; typedef struct DBusCallbackObject DBusCallbackObject; +typedef enum +{ + DBUS_HANDLER_RESULT_REMOVE_MESSAGE, /**< Remove this message, no further processing. */ + DBUS_HANDLER_RESULT_ALLOW_MORE_HANDLERS /**< Run any additional handlers that are interested in this message. */ +} DBusHandlerResult; + struct DBusObjectInfo { void *object_impl; /**< Object information */ diff --git a/dbus/dbus-objectid.c b/dbus/dbus-objectid.c index 1fb83e44..55ae0d48 100644 --- a/dbus/dbus-objectid.c +++ b/dbus/dbus-objectid.c @@ -177,6 +177,34 @@ dbus_object_id_set_low_bits (DBusObjectID *obj_id, #endif } +/** + * Set the object ID to an invalid value that cannot + * correspond to a valid object. + * + * @param obj_id the object ID + */ +void +dbus_object_id_set_null (DBusObjectID *obj_id) +{ + memset (obj_id, '\0', sizeof (DBusObjectID)); +} + +/** + * Check whether the object ID is set to a null value + * + * @param obj_id the object ID + * @returns #TRUE if null + */ +dbus_bool_t +dbus_object_id_is_null (const DBusObjectID *obj_id) +{ +#ifdef DBUS_HAVE_INT64 + return VALUE (obj_id) == 0; +#else + return HIGH_BITS (obj_id) == 0 && LOW_BITS (obj_id) == 0; +#endif +} + #ifdef DBUS_HAVE_INT64 /** * An object ID contains 64 bits of data. This function diff --git a/dbus/dbus-objectid.h b/dbus/dbus-objectid.h index b5e1f606..ad8ea1c5 100644 --- a/dbus/dbus-objectid.h +++ b/dbus/dbus-objectid.h @@ -54,6 +54,8 @@ void dbus_object_id_set_high_bits (DBusObjectID *obj_id dbus_uint32_t value); void dbus_object_id_set_low_bits (DBusObjectID *obj_id, dbus_uint32_t value); +void dbus_object_id_set_null (DBusObjectID *obj_id); +dbus_bool_t dbus_object_id_is_null (const DBusObjectID *obj_id); #ifdef DBUS_HAVE_INT64 dbus_uint64_t dbus_object_id_get_as_integer (const DBusObjectID *obj_id); void dbus_object_id_set_as_integer (DBusObjectID *obj_id, -- cgit