From b0ee0cbb46e9d550acfe5e784d46c6d0a8483a6a Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Tue, 21 Feb 2006 06:25:50 +0000 Subject: [PATCH] 2006-02-21 Dan Williams * gnome/libnm_glib/libnm_glib.c - Use __func__ everywhere we can - Code cleanups - Use dbus pending calls rather than blocking - Reduce busywaits for our thread to start and stop (gnome.org #330562) - (libnm_glib_dbus_init): Use dbus_bus_get_private() so we don't stomp on others using the default shared dbus connection. Fixes #rh177546# and gnome.org #326572 git-svn-id: http://svn-archive.gnome.org/svn/NetworkManager/trunk@1480 4912f4e0-d625-0410-9fb7-b9a5a253dbdc --- ChangeLog | 12 +++ gnome/libnm_glib/libnm_glib.c | 174 +++++++++++++++++++--------------- 2 files changed, 112 insertions(+), 74 deletions(-) diff --git a/ChangeLog b/ChangeLog index 8b132e4be8..b833e22229 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,15 @@ +2006-02-21 Dan Williams + + * gnome/libnm_glib/libnm_glib.c + - Use __func__ everywhere we can + - Code cleanups + - Use dbus pending calls rather than blocking + - Reduce busywaits for our thread to start and stop + (gnome.org #330562) + - (libnm_glib_dbus_init): Use dbus_bus_get_private() so we don't + stomp on others using the default shared dbus connection. + Fixes #rh177546# and gnome.org #326572 + 2006-02-21 Dan Williams Patch from Rodney Dawes diff --git a/gnome/libnm_glib/libnm_glib.c b/gnome/libnm_glib/libnm_glib.c index d5c79eb729..f3a5e4b8de 100644 --- a/gnome/libnm_glib/libnm_glib.c +++ b/gnome/libnm_glib/libnm_glib.c @@ -44,14 +44,14 @@ struct libnm_glib_ctx GSList * callbacks; GMutex * callbacks_lock; - guint callback_id_last; + guint callback_id_last; libnm_glib_state nm_state; }; typedef struct libnm_glib_callback { - guint id; + guint id; GMainContext * gmain_ctx; libnm_glib_ctx * libnm_glib_ctx; libnm_glib_callback_func func; @@ -61,52 +61,63 @@ typedef struct libnm_glib_callback static void libnm_glib_schedule_dbus_watcher (libnm_glib_ctx *ctx); static DBusConnection * libnm_glib_dbus_init (gpointer *user_data, GMainContext *context); +static void libnm_glib_update_state (libnm_glib_ctx *ctx, NMState state); - -static NMState libnm_glib_get_nm_state (DBusConnection *con) +static void +libnm_glib_nm_state_cb (DBusPendingCall *pcall, + void *user_data) { - DBusMessage * message; - DBusMessage * reply; - DBusError error; - NMState state = NM_STATE_UNKNOWN; + DBusMessage * reply; + libnm_glib_ctx * ctx = (libnm_glib_ctx *) user_data; + NMState nm_state; - g_return_val_if_fail (con != NULL, NM_STATE_UNKNOWN); + g_return_if_fail (pcall != NULL); + g_return_if_fail (ctx != NULL); - if (!(message = dbus_message_new_method_call (NM_DBUS_SERVICE, NM_DBUS_PATH, NM_DBUS_INTERFACE, "state"))) + if (!(reply = dbus_pending_call_steal_reply (pcall))) + goto out; + + if (dbus_message_get_type (reply) == DBUS_MESSAGE_TYPE_ERROR) { - fprintf (stderr, "libnm_glib_get_nm_state(): Couldn't allocate the dbus message\n"); - return NM_STATE_UNKNOWN; - } + DBusError err; - dbus_error_init (&error); - reply = dbus_connection_send_with_reply_and_block (con, message, -1, &error); - dbus_message_unref (message); - if (dbus_error_is_set (&error)) - { - fprintf (stderr, "libnm_glib_get_nm_state(): %s raised:\n %s\n\n", error.name, error.message); + dbus_error_init (&err); + dbus_set_error_from_message (&err, reply); + fprintf (stderr, "%s: dbus returned an error.\n (%s) %s\n", __func__, err.name, err.message); + dbus_error_free (&err); + dbus_message_unref (reply); goto out; } - if (!reply) - { - fprintf (stderr, "libnm_glib_get_nm_state(): dbus reply message was NULL\n" ); - goto out; - } - - dbus_error_init (&error); - if (!(dbus_message_get_args (reply, &error, DBUS_TYPE_UINT32, &state, DBUS_TYPE_INVALID))) - fprintf (stderr, "libnm_glib_get_nm_state(): error while getting args: name='%s' message='%s'\n", error.name, error.message); + if (dbus_message_get_args (reply, NULL, DBUS_TYPE_UINT32, &nm_state, DBUS_TYPE_INVALID)) + libnm_glib_update_state (ctx, nm_state); + dbus_message_unref (reply); out: - if (dbus_error_is_set (&error)) - dbus_error_free (&error); - return state; + dbus_pending_call_unref (pcall); } +static void +libnm_glib_get_nm_state (libnm_glib_ctx *ctx) +{ + DBusMessage * message; + DBusPendingCall * pcall = NULL; -static gboolean libnm_glib_callback_helper (gpointer user_data) + g_return_if_fail (ctx != NULL); + + if ((message = dbus_message_new_method_call (NM_DBUS_SERVICE, NM_DBUS_PATH, NM_DBUS_INTERFACE, "state"))) + { + dbus_connection_send_with_reply (ctx->dbus_con, message, &pcall, -1); + if (pcall) + dbus_pending_call_set_notify (pcall, libnm_glib_nm_state_cb, ctx, NULL); + dbus_message_unref (message); + } +} + +static gboolean +libnm_glib_callback_helper (gpointer user_data) { libnm_glib_callback *cb_data = (libnm_glib_callback *)user_data; @@ -116,10 +127,12 @@ static gboolean libnm_glib_callback_helper (gpointer user_data) (*(cb_data->func)) (cb_data->libnm_glib_ctx, cb_data->user_data); - return FALSE; /* never reschedule ourselves */ + return FALSE; } -static void libnm_glib_schedule_single_callback (libnm_glib_ctx *ctx, libnm_glib_callback *callback) +static void +libnm_glib_schedule_single_callback (libnm_glib_ctx *ctx, + libnm_glib_callback *callback) { GSource *source; @@ -134,7 +147,9 @@ static void libnm_glib_schedule_single_callback (libnm_glib_ctx *ctx, libnm_glib g_source_unref (source); } -static void libnm_glib_unschedule_single_callback (libnm_glib_ctx *ctx, libnm_glib_callback *callback) +static void +libnm_glib_unschedule_single_callback (libnm_glib_ctx *ctx, + libnm_glib_callback *callback) { GSource *source; @@ -146,7 +161,8 @@ static void libnm_glib_unschedule_single_callback (libnm_glib_ctx *ctx, libnm_gl g_source_destroy (source); } -static void libnm_glib_call_callbacks (libnm_glib_ctx *ctx) +static void +libnm_glib_call_callbacks (libnm_glib_ctx *ctx) { GSList *elem; @@ -163,7 +179,9 @@ static void libnm_glib_call_callbacks (libnm_glib_ctx *ctx) } -static void libnm_glib_update_state (libnm_glib_ctx *ctx, NMState state) +static void +libnm_glib_update_state (libnm_glib_ctx *ctx, + NMState state) { libnm_glib_state old_state; @@ -193,7 +211,10 @@ static void libnm_glib_update_state (libnm_glib_ctx *ctx, NMState state) } -static DBusHandlerResult libnm_glib_dbus_filter (DBusConnection *connection, DBusMessage *message, void *user_data) +static DBusHandlerResult +libnm_glib_dbus_filter (DBusConnection *connection, + DBusMessage *message, + void *user_data) { libnm_glib_ctx *ctx = (libnm_glib_ctx *)user_data; gboolean handled = TRUE; @@ -230,7 +251,7 @@ static DBusHandlerResult libnm_glib_dbus_filter (DBusConnection *connection, DBu gboolean new_owner_good = (new_owner && (strlen (new_owner) > 0)); if (!old_owner_good && new_owner_good) /* Equivalent to old ServiceCreated signal */ - libnm_glib_update_state (ctx, libnm_glib_get_nm_state (ctx->dbus_con)); + libnm_glib_get_nm_state (ctx); else if (old_owner_good && !new_owner_good) /* Equivalent to old ServiceDeleted signal */ ctx->nm_state = LIBNM_NO_NETWORKMANAGER; } @@ -241,7 +262,7 @@ static DBusHandlerResult libnm_glib_dbus_filter (DBusConnection *connection, DBu || dbus_message_is_signal (message, NM_DBUS_INTERFACE, "DeviceActivating") || dbus_message_is_signal (message, NM_DBUS_INTERFACE, "DevicesChanged")) { - libnm_glib_update_state (ctx, libnm_glib_get_nm_state (ctx->dbus_con)); + libnm_glib_get_nm_state (ctx); } else if (dbus_message_is_signal (message, NM_DBUS_INTERFACE, NM_DBUS_SIGNAL_STATE_CHANGE)) { @@ -266,16 +287,18 @@ static DBusHandlerResult libnm_glib_dbus_filter (DBusConnection *connection, DBu * Initialize a connection to dbus and set up our callbacks. * */ -static DBusConnection * libnm_glib_dbus_init (gpointer *user_data, GMainContext *context) +static DBusConnection * +libnm_glib_dbus_init (gpointer *user_data, + GMainContext *context) { DBusConnection *connection = NULL; DBusError error; dbus_error_init (&error); - connection = dbus_bus_get (DBUS_BUS_SYSTEM, &error); + connection = dbus_bus_get_private (DBUS_BUS_SYSTEM, &error); if (dbus_error_is_set (&error)) { - fprintf (stderr, "libnm: error, %s raised:\n %s\n\n", error.name, error.message); + fprintf (stderr, "%s: error, %s raised:\n %s\n\n", __func__, error.name, error.message); dbus_error_free (&error); return (NULL); } @@ -289,7 +312,7 @@ static DBusConnection * libnm_glib_dbus_init (gpointer *user_data, GMainContext dbus_connection_setup_with_g_main (connection, context); dbus_error_init (&error); - dbus_bus_add_match(connection, + dbus_bus_add_match (connection, "type='signal'," "interface='" DBUS_INTERFACE_DBUS "'," "sender='" DBUS_SERVICE_DBUS "'", @@ -298,7 +321,7 @@ static DBusConnection * libnm_glib_dbus_init (gpointer *user_data, GMainContext dbus_error_free (&error); dbus_error_init (&error); - dbus_bus_add_match(connection, + dbus_bus_add_match (connection, "type='signal'," "interface='" NM_DBUS_INTERFACE "'," "path='" NM_DBUS_PATH "'," @@ -317,7 +340,8 @@ static DBusConnection * libnm_glib_dbus_init (gpointer *user_data, GMainContext * Repeatedly try to re-activate the connection to dbus. * */ -static gboolean libnm_glib_dbus_watcher (gpointer user_data) +static gboolean +libnm_glib_dbus_watcher (gpointer user_data) { libnm_glib_ctx *ctx = (libnm_glib_ctx *)user_data; @@ -341,14 +365,15 @@ static gboolean libnm_glib_dbus_watcher (gpointer user_data) * attempt to re-activate the dbus connection until connected. * */ -static void libnm_glib_schedule_dbus_watcher (libnm_glib_ctx *ctx) +static void +libnm_glib_schedule_dbus_watcher (libnm_glib_ctx *ctx) { g_return_if_fail (ctx != NULL); if (ctx->dbus_watcher == 0) { GSource *source = g_idle_source_new (); - g_source_set_callback (source, libnm_glib_dbus_watcher, (gpointer)ctx, NULL); + g_source_set_callback (source, libnm_glib_dbus_watcher, (gpointer) ctx, NULL); ctx->dbus_watcher = g_source_attach (source, ctx->g_main_ctx); g_source_unref (source); } @@ -361,7 +386,8 @@ static void libnm_glib_schedule_dbus_watcher (libnm_glib_ctx *ctx) * Main thread for libnm * */ -static gpointer libnm_glib_dbus_worker (gpointer user_data) +static gpointer +libnm_glib_dbus_worker (gpointer user_data) { libnm_glib_ctx *ctx = (libnm_glib_ctx *)user_data; @@ -372,13 +398,10 @@ static gpointer libnm_glib_dbus_worker (gpointer user_data) * down. Should probably be done by a timeout polling dbus_connection_is_connected() * or by getting connection status out of libdbus or something. */ - if (!ctx->dbus_con) + if (!(ctx->dbus_con = libnm_glib_dbus_init ((gpointer) ctx, ctx->g_main_ctx))) libnm_glib_schedule_dbus_watcher (ctx); else - { - /* Get initial status */ - libnm_glib_update_state (ctx, libnm_glib_get_nm_state (ctx->dbus_con)); - } + libnm_glib_get_nm_state (ctx); ctx->thread_inited = TRUE; g_main_loop_run (ctx->g_main_loop); @@ -388,13 +411,14 @@ static gpointer libnm_glib_dbus_worker (gpointer user_data) } -static void libnm_glib_ctx_free (libnm_glib_ctx *ctx) +static void +libnm_glib_ctx_free (libnm_glib_ctx *ctx) { g_return_if_fail (ctx != NULL); if (ctx->check == 0xDD) { - fprintf (stderr, "libnm_glib_ctx_free(): context %p already freed!\n", ctx); + fprintf (stderr, "%s: context %p already freed!\n", __func__, ctx); return; } @@ -418,7 +442,8 @@ static void libnm_glib_ctx_free (libnm_glib_ctx *ctx) } -static libnm_glib_ctx *libnm_glib_ctx_new (void) +static libnm_glib_ctx * +libnm_glib_ctx_new (void) { libnm_glib_ctx *ctx = g_malloc0 (sizeof (libnm_glib_ctx)); @@ -437,7 +462,8 @@ error: } -libnm_glib_ctx *libnm_glib_init (void) +libnm_glib_ctx * +libnm_glib_init (void) { GError *error = NULL; libnm_glib_ctx *ctx = NULL; @@ -447,17 +473,9 @@ libnm_glib_ctx *libnm_glib_init (void) g_thread_init (NULL); dbus_g_thread_init (); - ctx = libnm_glib_ctx_new(); - if (!ctx) + if (!(ctx = libnm_glib_ctx_new())) return NULL; - /* We don't care if dbus isn't around yet, we keep checking for it and - * intialize our connection when it comes up. - */ - ctx->dbus_con = libnm_glib_dbus_init ((gpointer)ctx, ctx->g_main_ctx); - if (ctx->dbus_con) - libnm_glib_update_state (ctx, libnm_glib_get_nm_state (ctx->dbus_con)); - if (!g_thread_create (libnm_glib_dbus_worker, ctx, FALSE, &error)) { if (error) @@ -467,7 +485,7 @@ libnm_glib_ctx *libnm_glib_init (void) /* Wait until initialization of the thread */ while (!ctx->thread_inited) - g_usleep (G_USEC_PER_SEC / 2); + g_usleep (G_USEC_PER_SEC / 20); return ctx; @@ -477,19 +495,21 @@ error: } -void libnm_glib_shutdown (libnm_glib_ctx *ctx) +void +libnm_glib_shutdown (libnm_glib_ctx *ctx) { g_return_if_fail (ctx != NULL); g_main_loop_quit (ctx->g_main_loop); - while (!(ctx->thread_done)) - g_usleep (G_USEC_PER_SEC / 2); + while (!ctx->thread_done) + g_usleep (G_USEC_PER_SEC / 20); libnm_glib_ctx_free (ctx); } -libnm_glib_state libnm_glib_get_network_state (const libnm_glib_ctx *ctx) +libnm_glib_state +libnm_glib_get_network_state (const libnm_glib_ctx *ctx) { if (!ctx) return LIBNM_INVALID_CONTEXT; @@ -498,7 +518,11 @@ libnm_glib_state libnm_glib_get_network_state (const libnm_glib_ctx *ctx) } -guint libnm_glib_register_callback (libnm_glib_ctx *ctx, libnm_glib_callback_func func, gpointer user_data, GMainContext *g_main_ctx) +guint +libnm_glib_register_callback (libnm_glib_ctx *ctx, + libnm_glib_callback_func func, + gpointer user_data, + GMainContext *g_main_ctx) { libnm_glib_callback *callback = NULL; @@ -522,7 +546,9 @@ guint libnm_glib_register_callback (libnm_glib_ctx *ctx, libnm_glib_callback_fun } -void libnm_glib_unregister_callback (libnm_glib_ctx *ctx, guint id) +void +libnm_glib_unregister_callback (libnm_glib_ctx *ctx, + guint id) { GSList *elem;