On Tue, Aug 02, 2016 at 02:01:07AM +0530, Atul Anand wrote: > A new object NMPacRunnerManager has been added to manage and interact > PacRunner. It invokes both DBus methods on PacRunner DBus interface. > It stores the returned object path from CreateProxyConfiguration() > to feed as parameter to DestroyProxyCofiguration() when network goes down.
[...]
> +static void
> +pacrunner_proxy_cb (GObject *source, GAsyncResult *res, gpointer user_data)
> +{
> + NMPacRunnerManager *self = NULL;
> + NMPacRunnerManagerPrivate *priv;
> + gs_free_error GError *error = NULL;
> + GDBusProxy *proxy;
> +
> + self = NM_PACRUNNER_MANAGER (user_data);
> + priv = NM_PACRUNNER_MANAGER_GET_PRIVATE (self);
> +
> + proxy = g_dbus_proxy_new_for_bus_finish (res, &error);
> + if (!proxy && g_error_matches (error, G_IO_ERROR,
> G_IO_ERROR_CANCELLED)) {
> + /* Mark PacRunner unavailable on DBus */
> + priv->started = FALSE;
> + _LOGD ("failed to connect to pacrunner via DBus: %s",
> error->message);
> + return;
> + }
We should handle all errors, not only CANCELLED. For example:
if (!proxy) {
if (!g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED))
_LOGW ("failed to connect to pacrunner via D-Bus: %s",
error->message);
g_clear_error (&error);
return;
}
> +static void
> +start_pacrunner (NMPacRunnerManager *self)
> +{
> + NMPacRunnerManagerPrivate *priv = NM_PACRUNNER_MANAGER_GET_PRIVATE
> (self);
> +
> + if (priv->started)
> + return;
> +
> + priv->pacrunner_cancellable = g_cancellable_new ();
> +
> + g_dbus_proxy_new_for_bus (G_BUS_TYPE_SYSTEM,
> + G_DBUS_PROXY_FLAGS_DO_NOT_AUTO_START,
At least on Fedora, pacrunner seems to be D-Bus activated and so the
method invocation fails if the service is not running. Maybe we should
remove the G_DBUS_PROXY_FLAGS_DO_NOT_AUTO_START flag, so that it's
started when needed?
> + NULL,
> + PACRUNNER_DBUS_SERVICE,
> + PACRUNNER_DBUS_PATH,
> + PACRUNNER_DBUS_INTERFACE,
> + priv->pacrunner_cancellable,
> + (GAsyncReadyCallback) pacrunner_proxy_cb,
> + self);
> + priv->started = TRUE;
> +}
> +static void
> +pacrunner_remove_done (GObject *source, GAsyncResult *res, gpointer
> user_data)
> +{
> + NMPacRunnerManager *self = NM_PACRUNNER_MANAGER (user_data);
> + NMPacRunnerManagerPrivate *priv = NM_PACRUNNER_MANAGER_GET_PRIVATE
> (self);
> + gs_free_error GError *error = NULL;
> + gs_unref_variant GVariant *ret = NULL;
> +
> + ret = g_dbus_proxy_call_finish (priv->pacrunner, res, &error);
> + if (!ret || g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED))
I think the second condition after || can never be true because @error
is always NULL when @ret is non-NULL.
> + _LOGD ("Couldn't remove proxy config from pacrunner");
> + else
> + _LOGD ("Sucessfully removed proxy config from pacrunner");
> +}
> +
> +/**
> + * nm_pacrunner_manager_remove():
> + * @self: the #NMPacRunnerManager
> + * @iface: the iface for the connection to be removed
> + * from PacRunner
> + */
> +void
> +nm_pacrunner_manager_remove (NMPacRunnerManager *self, const char *iface)
> +{
> + NMPacRunnerManagerPrivate *priv = NM_PACRUNNER_MANAGER_GET_PRIVATE
> (self);
> + GList *list;
> +
> + for (list = g_list_first(priv->remove); list; list = g_list_next(list))
> {
> + struct remove_data *data = list->data;
> + if (g_strcmp0 (data->iface, iface) == 0) {
> + if ((priv->pacrunner) && (data->path))
You can remove the inner parenthesis.
Beniamino
signature.asc
Description: PGP signature
_______________________________________________ networkmanager-list mailing list [email protected] https://mail.gnome.org/mailman/listinfo/networkmanager-list
