Hi, On Fri, Apr 29, 2016 at 02:52:03PM -0500, Jonathon Jongsma wrote: > On Thu, 2016-04-28 at 15:37 +0200, Victor Toso wrote: > > Implementing the function that was introduced in previous commit: > > session_info_is_user() > > > > In console-kit we can gather the session type with GetSessionType api > > from Session interface. This function is not well documented but seems > > reliable enough as GDM was using it [0] > > > > [0] session_is_login_window on gui/simple-greeter/gdm-user-manager.c > > > > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1323640 > > --- > > src/console-kit.c | 68 +++++++++++++++++++++++++++++++++++++++++++++++++++++- > > - > > 1 file changed, 66 insertions(+), 2 deletions(-) > > > > diff --git a/src/console-kit.c b/src/console-kit.c > > index 036234e..182e886 100644 > > --- a/src/console-kit.c > > +++ b/src/console-kit.c > > @@ -503,8 +503,72 @@ gboolean session_info_session_is_locked(struct > > session_info *info) > > return (info->session_is_locked || info->session_idle_hint); > > } > > > > +/* This function should only be called after session_info_get_active_session > > + * in order to verify if active session belongs to user (non greeter) */ > > gboolean session_info_is_user(struct session_info *info) > > { > > - /* TODO */ > > - return TRUE; > > + DBusError error; > > + DBusMessage *message; > > + DBusMessage *reply; > > would prefer to initialize message and reply to NULL just in case at some point > the code changes and there is a 'goto exit' call before one of these variables > gets assigned. Sure, I'll change it. > > > + gchar *session_type = NULL; > > + gboolean ret = TRUE; > > + > > + if (info == NULL || info->connection == NULL || info->active_session == > > NULL) > > + return TRUE; > > Here again, we're defaulting to permissive. As in the previous reviews, should > we default to restrictive? Maybe not, but I just wanted to mention it. Sure. Let me know what you think from my replies in previous patches. > > + > > + message = dbus_message_new_method_call(INTERFACE_CONSOLE_KIT, > > + info->active_session, > > + INTERFACE_CONSOLE_KIT_SESSION, > > + "GetSessionType"); > > + if (message == NULL) { > > + syslog(LOG_ERR, > > + "(console-kit) Unable to create dbus message for > > GetSessionType"); > > + return TRUE; > > + } > > + > > + dbus_error_init(&error); > > + reply = dbus_connection_send_with_reply_and_block(info->connection, > > + message, > > + -1, > > + &error); > > + if (reply == NULL || dbus_error_is_set(&error)) { > > + if (dbus_error_is_set(&error)) { > > + syslog(LOG_ERR, "GetSessionType failed: %s", error.message); > > + dbus_error_free(&error); > > + } else > > + syslog(LOG_ERR, "GetSessionType failed"); > > + goto exit; > > + } > > + > > + dbus_error_init(&error); > > + if (!dbus_message_get_args(reply, > > + &error, > > + DBUS_TYPE_STRING, &session_type, > > + DBUS_TYPE_INVALID)) { > > + if (dbus_error_is_set(&error)) { > > + syslog(LOG_ERR, > > + "(console-kit) fail to get session-type from reply: %s", > > + error.message); > > + dbus_error_free(&error); > > + } else { > > + syslog(LOG_ERR, "(console-kit) fail to get session-type from > > reply"); > > + } > > + session_type = NULL; > > + goto exit; > > + } > > + > > + /* Empty session_type means user */ > > + if (info->verbose) > > + syslog(LOG_DEBUG, "(console-kit) session-type is '%s'", > > session_type); > > + > > + ret = (g_strcmp0 (session_type, "LoginWindow") != 0); > > + > > +exit: > > + if (reply != NULL) { > > + dbus_message_unref(reply); > > + } > > + if (message != NULL) { > > + dbus_message_unref(message); > > + } > > + return ret; > > } > > Otherwise it looks ok. > > Reviewed-by: Jonathon Jongsma <jjongsma@xxxxxxxxxx> Many thanks for the reviews. I'll send the v4 after we decide if we should change the session_info_session_is_locked to return TRUE instead of FALSE as default and session_info_is_user to return FALSE instead of TRUE as default. Cheers, toso > _______________________________________________ > 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