Hi, On Fri, Apr 29, 2016 at 10:08:53AM -0500, Jonathon Jongsma wrote: > On Thu, 2016-04-28 at 15:37 +0200, Victor Toso wrote: > > We register to read the Lock, Unlock and IdleHintChanged signals from > > ConsoleKit.Session. The Lock/Unlock signals should be the right > > signals for the job but not all Desktop Managers implement its locking > > in a way that trigger those signals. That's why we double-check with > > IdleHintChanged signal that it might be triggered by other services > > like screen savers. > > > > Acked-by: Jonathon Jongsma <jjongsma@xxxxxxxxxx> > > --- > > src/console-kit.c | 85 +++++++++++++++++++++++++++++++++++++++++++++++++++++- > > - > > 1 file changed, 82 insertions(+), 3 deletions(-) > > > > diff --git a/src/console-kit.c b/src/console-kit.c > > index 72a5b16..b21df6f 100644 > > --- a/src/console-kit.c > > +++ b/src/console-kit.c > > @@ -35,6 +35,9 @@ struct session_info { > > char *active_session; > > int verbose; > > gchar *match_seat_signals; > > + gchar *match_session_signals; > > + gboolean session_is_locked; > > + gboolean session_idle_hint; > > }; > > > > #define INTERFACE_CONSOLE_KIT "org.freedesktop.ConsoleKit" > > @@ -45,8 +48,15 @@ struct session_info { > > > > #define INTERFACE_CONSOLE_KIT_SEAT INTERFACE_CONSOLE_KIT ".Seat" > > > > +#define INTERFACE_CONSOLE_KIT_SESSION INTERFACE_CONSOLE_KIT ".Session" > > +#define OBJ_PATH_CONSOLE_KIT_SESSION OBJ_PATH_CONSOLE_KIT "/Session" > > + > > #define SEAT_SIGNAL_ACTIVE_SESSION_CHANGED "ActiveSessionChanged" > > > > +#define SESSION_SIGNAL_LOCK "Lock" > > +#define SESSION_SIGNAL_UNLOCK "Unlock" > > +#define SESSION_SIGNAL_IDLE_HINT_CHANGED "IdleHintChanged" > > + > > static char *console_kit_get_first_seat(struct session_info *info); > > static char *console_kit_check_active_session_change(struct session_info > > *info); > > > > @@ -64,6 +74,19 @@ static void si_dbus_match_remove(struct session_info *info) > > g_free(info->match_seat_signals); > > info->match_seat_signals = NULL; > > } > > + > > + if (info->match_session_signals != NULL) { > > + dbus_error_init(&error); > > + dbus_bus_remove_match(info->connection, > > + info->match_session_signals, > > + &error); > > + > > + if (info->verbose) > > + syslog(LOG_DEBUG, "(console-kit) session match removed: %s", > > + info->match_session_signals); > > + g_free(info->match_session_signals); > > + info->match_session_signals = NULL; > > + } > > } > > > > static void si_dbus_match_rule_update(struct session_info *info) > > @@ -97,6 +120,28 @@ static void si_dbus_match_rule_update(struct session_info > > *info) > > g_free(info->match_seat_signals); > > } > > } > > + > > + /* Session signals */ > > + if (info->active_session != NULL) { > > + info->match_session_signals = > > + g_strdup_printf ("type='signal',interface='%s',path='%s'", > > + INTERFACE_CONSOLE_KIT_SESSION, > > + info->active_session); > > + if (info->verbose) > > + syslog(LOG_DEBUG, "(console-kit) session match: %s", > > + info->match_session_signals); > > + > > + dbus_error_init(&error); > > + dbus_bus_add_match(info->connection, > > + info->match_session_signals, > > + &error); > > + if (dbus_error_is_set(&error)) { > > + syslog(LOG_WARNING, "Unable to add dbus rule match: %s", > > + error.message); > > + dbus_error_free(&error); > > + g_free(info->match_session_signals); > > Dangling pointer. Set to NULL. Ok! > > > + } > > + } > > } > > > > static void > > @@ -133,6 +178,7 @@ si_dbus_read_signals(struct session_info *info) > > dbus_message_iter_get_basic(&iter, &session); > > if (session != NULL && session[0] != '\0') { > > info->active_session = g_strdup(session); > > + si_dbus_match_rule_update(info); > > } else { > > syslog(LOG_WARNING, "(console-kit) received invalid > > session. " > > "No active-session at the moment"); > > @@ -142,6 +188,25 @@ si_dbus_read_signals(struct session_info *info) > > "ActiveSessionChanged message has unexpected type: > > '%c'", > > type); > > } > > + } else if (g_strcmp0(member, SESSION_SIGNAL_LOCK) == 0) { > > + info->session_is_locked = TRUE; > > + } else if (g_strcmp0(member, SESSION_SIGNAL_UNLOCK) == 0) { > > + info->session_is_locked = FALSE; > > + } else if (g_strcmp0(member, SESSION_SIGNAL_IDLE_HINT_CHANGED) == 0) > > { > > + DBusMessageIter iter; > > + gint type; > > + dbus_bool_t idle_hint; > > + > > + dbus_message_iter_init(message, &iter); > > + type = dbus_message_iter_get_arg_type(&iter); > > + if (type == DBUS_TYPE_BOOLEAN) { > > + dbus_message_iter_get_basic(&iter, &idle_hint); > > + info->session_idle_hint = (idle_hint); > > + } else { > > + syslog(LOG_ERR, > > + "(console-kit) IdleHintChanged has unexpected type: > > '%c'", > > + type); > > + } > > } else if (info->verbose) { > > syslog(LOG_DEBUG, "(console-kit) Signal not handled: %s", > > member); > > } > > @@ -162,6 +227,8 @@ struct session_info *session_info_create(int verbose) > > return NULL; > > > > info->verbose = verbose; > > + info->session_is_locked = FALSE; > > + info->session_idle_hint = FALSE; > > > > dbus_error_init(&error); > > info->connection = dbus_bus_get_private(DBUS_BUS_SYSTEM, &error); > > @@ -324,6 +391,7 @@ const char *session_info_get_active_session(struct > > session_info *info) > > } > > > > info->active_session = strdup(session); > > + si_dbus_match_rule_update(info); > > > > exit: > > if (reply != NULL) { > > @@ -419,7 +487,18 @@ static char > > *console_kit_check_active_session_change(struct session_info *info) > > > > gboolean session_info_session_is_locked(struct session_info *info) > > { > > - /* TODO: It could be implemented based on Lock/Unlock signals from > > Session > > - * interface. */ > > - return FALSE; > > + if (info == NULL) > > + return FALSE; > > This seems like an unexpected situation? Perhaps use something like > g_return_val_if_fail()? Agreed! > Do we really want to default to reporting 'unlocked' in this scenario? > reporting it as 'locked' seems somewhat safer The reason is that I was a bit afraid of changing the behavior in case of unforeseen problems. As this should only happen in a locked-screen and this scenario is likely a corner case in the usage of the file-xfer (and possible features that we want to disable in the future), I think defaulting to 'unlocked' is somewhat safer but I could be wrong. > > > + > > + /* Not every system does emit Lock and Unlock signals (for instance, such > > + * is the case for RHEL6) but most of the systems seems to emit the > > + * IdleHintChanged. So, we give priority to the Lock signal, if it is > > Locked > > + * we return that the session is locked, otherwise we double check with > > the > > + * IdleHint value */ > > + si_dbus_read_signals(info); > > + if (info->verbose) { > > + syslog(LOG_DEBUG, "(console-kit) session is locked: %s", > > + (info->session_is_locked || info->session_idle_hint) ? "yes" : > > "no"); > > + } > > + return (info->session_is_locked || info->session_idle_hint); > > Personal preference. I'd introduce a local boolean variable here so that you > don't need to repeat the (info->session_is_locked || info->session_idle_hint) > twice (once for the debug statement and once for the return value) I'll do that! toso > > > Reviewed-by: Jonathon Jongsma <jjongsma@xxxxxxxxxx> > _______________________________________________ > Spice-devel mailing list > Spice-devel@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/spice-devel _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel