change the connector API a bit + misc anality

I modified the API of the new connector a bit.
 * include errors
 * make names a bit more consistent with what they do
 * have symmetry between open and close
 * use refcounting
 * do parameter checking on inputs

I also made some changes to whitespace and style.  And converted
descriptions to gtkdoc style comments.

3 files changed, 762 insertions, 519 deletions

diff --git a/libck-connector/ck-connector.c b/libck-connector/ck-connector.c
index b22f76c..ebd8cf7 100644
--- a/libck-connector/ck-connector.c
+++ b/libck-connector/ck-connector.c
@@ -1,8 +1,10 @@
-/*
+/* -*- Mode: C; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 8 -*-
+ *
  * ck-connector.c : Code for login managers to register with ConsoleKit.
  *
  * Copyright (c) 2007 David Zeuthen <davidz@redhat.com>
- * 
+ * Copyright (c) 2007 William Jon McCann <mccann@jhu.edu>
+ *
  * Permission is hereby granted, free of charge, to any person
  * obtaining a copy of this software and associated documentation
  * files (the "Software"), to deal in the Software without
@@ -11,10 +13,10 @@
  * copies of the Software, and to permit persons to whom the
  * Software is furnished to do so, subject to the following
  * conditions:
- * 
+ *
  * The above copyright notice and this permission notice shall be
  * included in all copies or substantial portions of the Software.
- * 
+ *
  * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
  * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
  * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
@@ -34,348 +36,620 @@
 
 #include "ck-connector.h"
 
+#if defined (__STDC_VERSION__) && (__STDC_VERSION__ >= 199901L)
+#define _CK_FUNCTION_NAME __func__
+#elif defined(__GNUC__) || defined(_MSC_VER)
+#define _CK_FUNCTION_NAME __FUNCTION__
+#else
+#define _CK_FUNCTION_NAME "unknown function"
+#endif
+
+#define CK_CONNECTOR_ERROR "org.freedesktop.CkConnector.Error"
+
+#define _CK_WARNING_FORMAT "arguments to %s() were incorrect, assertion \"%s\" failed in file %s line %d.\n"
+#define _ck_return_if_fail(condition) do {                                         \
+  if (!(condition)) {                                                              \
+          fprintf (stderr, _CK_WARNING_FORMAT, _CK_FUNCTION_NAME, #condition, __FILE__, __LINE__); \
+    return;                                                                        \
+  } } while (0)
+
+#define _ck_return_val_if_fail(condition, val) do {                                     \
+  if (!(condition)) {                                                              \
+          fprintf (stderr, _CK_WARNING_FORMAT, _CK_FUNCTION_NAME, #condition, __FILE__, __LINE__); \
+    return val;                                                                        \
+  } } while (0)
+
+struct _CkConnector
+{
+        int             refcount;
+        char           *cookie;
+        dbus_bool_t     session_created;
+        DBusConnection *connection;
+};
+
 static dbus_bool_t
-add_param_int32 (DBusMessageIter *iter_array, const char *key, dbus_int32_t value)
+add_param_int32 (DBusMessageIter *iter_array,
+                 const char      *key,
+                 dbus_int32_t     value)
 {
-	DBusMessageIter iter_struct;
-	DBusMessageIter iter_variant;
-
-	if (!dbus_message_iter_open_container (iter_array,
-					       DBUS_TYPE_STRUCT,
-					       NULL,
-					       &iter_struct))
-		goto oom;
-	if (!dbus_message_iter_append_basic (&iter_struct, DBUS_TYPE_STRING, &key))
-		goto oom;
-	if (!dbus_message_iter_open_container (&iter_struct,
-					       DBUS_TYPE_VARIANT,
-					       DBUS_TYPE_INT32_AS_STRING,
-					       &iter_variant))
-		goto oom;
-	if (!dbus_message_iter_append_basic (&iter_variant, DBUS_TYPE_INT32, &value))
-		goto oom;
-	if (!dbus_message_iter_close_container (&iter_struct, &iter_variant))
-		goto oom;
-	if (!dbus_message_iter_close_container (iter_array, &iter_struct))
-		goto oom;
-
-	return TRUE;
+        DBusMessageIter iter_struct;
+        DBusMessageIter iter_variant;
+
+        if (! dbus_message_iter_open_container (iter_array,
+                                                DBUS_TYPE_STRUCT,
+                                                NULL,
+                                                &iter_struct)) {
+                goto oom;
+        }
+
+        if (! dbus_message_iter_append_basic (&iter_struct,
+                                              DBUS_TYPE_STRING,
+                                              &key)) {
+                goto oom;
+        }
+
+        if (! dbus_message_iter_open_container (&iter_struct,
+                                                DBUS_TYPE_VARIANT,
+                                                DBUS_TYPE_INT32_AS_STRING,
+                                                &iter_variant)) {
+                goto oom;
+        }
+
+        if (! dbus_message_iter_append_basic (&iter_variant,
+                                              DBUS_TYPE_INT32,
+                                              &value)) {
+                goto oom;
+        }
+
+        if (! dbus_message_iter_close_container (&iter_struct,
+                                                 &iter_variant)) {
+                goto oom;
+        }
+
+        if (! dbus_message_iter_close_container (iter_array,
+                                                 &iter_struct)) {
+                goto oom;
+        }
+
+        return TRUE;
 oom:
-	return FALSE;
+        return FALSE;
 }
 
 static dbus_bool_t
-add_param_string (DBusMessageIter *iter_array, const char *key, const char *value)
+add_param_string (DBusMessageIter *iter_array,
+                  const char      *key,
+                  const char      *value)
 {
-	DBusMessageIter iter_struct;
-	DBusMessageIter iter_variant;
-
-	if (!dbus_message_iter_open_container (iter_array,
-					       DBUS_TYPE_STRUCT,
-					       NULL,
-					       &iter_struct))
-		goto oom;
-	if (!dbus_message_iter_append_basic (&iter_struct, DBUS_TYPE_STRING, &key))
-		goto oom;
-	if (!dbus_message_iter_open_container (&iter_struct,
-					       DBUS_TYPE_VARIANT,
-					       DBUS_TYPE_STRING_AS_STRING,
-					       &iter_variant))
-		goto oom;
-	if (!dbus_message_iter_append_basic (&iter_variant, DBUS_TYPE_STRING, &value))
-		goto oom;
-	if (!dbus_message_iter_close_container (&iter_struct, &iter_variant))
-		goto oom;
-	if (!dbus_message_iter_close_container (iter_array, &iter_struct))
-		goto oom;
-
-	return TRUE;
+        DBusMessageIter iter_struct;
+        DBusMessageIter iter_variant;
+
+        if (! dbus_message_iter_open_container (iter_array,
+                                                DBUS_TYPE_STRUCT,
+                                                NULL,
+                                                &iter_struct)) {
+                goto oom;
+        }
+
+        if (! dbus_message_iter_append_basic (&iter_struct,
+                                              DBUS_TYPE_STRING,
+                                              &key)) {
+                goto oom;
+        }
+
+        if (! dbus_message_iter_open_container (&iter_struct,
+                                                DBUS_TYPE_VARIANT,
+                                                DBUS_TYPE_STRING_AS_STRING,
+                                                &iter_variant)) {
+                goto oom;
+        }
+
+        if (! dbus_message_iter_append_basic (&iter_variant,
+                                              DBUS_TYPE_STRING,
+                                              &value)) {
+                goto oom;
+        }
+
+        if (! dbus_message_iter_close_container (&iter_struct,
+                                                 &iter_variant)) {
+                goto oom;
+        }
+
+        if (! dbus_message_iter_close_container (iter_array,
+                                                 &iter_struct)) {
+                goto oom;
+        }
+
+        return TRUE;
 oom:
-	return FALSE;
+        return FALSE;
 }
 
 static dbus_bool_t
-add_param_bool (DBusMessageIter *iter_array, const char *key, dbus_bool_t value)
+add_param_bool (DBusMessageIter *iter_array,
+                const char      *key,
+                dbus_bool_t      value)
 {
-	DBusMessageIter iter_struct;
-	DBusMessageIter iter_variant;
-
-	if (!dbus_message_iter_open_container (iter_array,
-					       DBUS_TYPE_STRUCT,
-					       NULL,
-					       &iter_struct))
-		goto oom;
-	if (!dbus_message_iter_append_basic (&iter_struct, DBUS_TYPE_STRING, &key))
-		goto oom;
-	if (!dbus_message_iter_open_container (&iter_struct,
-					       DBUS_TYPE_VARIANT,
-					       DBUS_TYPE_BOOLEAN_AS_STRING,
-					       &iter_variant))
-		goto oom;
-	if (!dbus_message_iter_append_basic (&iter_variant, DBUS_TYPE_BOOLEAN, &value))
-		goto oom;
-	if (!dbus_message_iter_close_container (&iter_struct, &iter_variant))
-		goto oom;
-	if (!dbus_message_iter_close_container (iter_array, &iter_struct))
-		goto oom;
-
-	return TRUE;
+        DBusMessageIter iter_struct;
+        DBusMessageIter iter_variant;
+
+        if (! dbus_message_iter_open_container (iter_array,
+                                                DBUS_TYPE_STRUCT,
+                                                NULL,
+                                                &iter_struct)) {
+                goto oom;
+        }
+
+        if (! dbus_message_iter_append_basic (&iter_struct,
+                                              DBUS_TYPE_STRING,
+                                              &key)) {
+                goto oom;
+        }
+
+        if (! dbus_message_iter_open_container (&iter_struct,
+                                                DBUS_TYPE_VARIANT,
+                                                DBUS_TYPE_BOOLEAN_AS_STRING,
+                                                &iter_variant)) {
+                goto oom;
+        }
+
+        if (! dbus_message_iter_append_basic (&iter_variant,
+                                              DBUS_TYPE_BOOLEAN,
+                                              &value)) {
+                goto oom;
+        }
+
+        if (! dbus_message_iter_close_container (&iter_struct,
+                                                 &iter_variant)) {
+                goto oom;
+        }
+
+        if (! dbus_message_iter_close_container (iter_array,
+                                                 &iter_struct)) {
+                goto oom;
+        }
+
+        return TRUE;
 oom:
-	return FALSE;
+        return FALSE;
+}
+
+/* Frees all resources allocated and disconnects from the system
+ * message bus.
+ */
+static void
+_ck_connector_free (CkConnector *connector)
+{
+        if (connector->connection != NULL) {
+                /* it's a private connection so it's all good */
+                dbus_connection_close (connector->connection);
+        }
+
+        if (connector->cookie != NULL) {
+                free (connector->cookie);
+        }
+
+        free (connector);
 }
 
+/**
+ * Decrements the reference count of a CkConnector, disconnecting
+ * from the bus and freeing the connector if the count reaches 0.
+ *
+ * @param connector the connector
+ * @see ck_connector_ref
+ */
+void
+ck_connector_unref (CkConnector *connector)
+{
+        _ck_return_if_fail (connector != NULL);
 
-struct CKConnector_s
+        /* Probably should use some kind of atomic op here */
+        connector->refcount -= 1;
+        if (connector->refcount == 0) {
+                _ck_connector_free (connector);
+        }
+}
+
+/**
+ * Increments the reference count of a CkConnector.
+ *
+ * @param connector the connector
+ * @returns the connector
+ * @see ck_connector_unref
+ */
+CkConnector *
+ck_connector_ref (CkConnector *connector)
 {
-	char *cookie;
-	dbus_bool_t session_created;
-	DBusConnection *con;
-};
+        _ck_return_val_if_fail (connector != NULL, NULL);
+
+        /* Probably should use some kind of atomic op here */
+        connector->refcount += 1;
 
-CKConnector*
-ckc_new (void)
+        return connector;
+}
+
+/**
+ * Constructs a new Connector to communicate with the ConsoleKit
+ * daemon. Returns #NULL if memory can't be allocated for the
+ * object.
+ *
+ * @returns a new CkConnector, free with ck_connector_unref()
+ */
+CkConnector *
+ck_connector_new (void)
 {
-	CKConnector *ckc;
-
-	ckc = calloc (1, sizeof (CKConnector));
-	if (ckc == NULL)
-		goto oom;
-	ckc->con = NULL;
-	ckc->cookie = NULL;
-	ckc->session_created = FALSE;
+        CkConnector *connector;
+
+        connector = calloc (1, sizeof (CkConnector));
+        if (connector == NULL) {
+                goto oom;
+        }
+
+        connector->refcount = 1;
+        connector->connection = NULL;
+        connector->cookie = NULL;
+        connector->session_created = FALSE;
 oom:
-	return ckc;
-};
+        return connector;
+}
 
+/**
+ * Connects to the D-Bus system bus daemon and issues the method call
+ * OpenSession on the ConsoleKit manager interface. The
+ * connection to the bus is private.
+ *
+ * Returns FALSE on OOM, if the system bus daemon is not running, if
+ * the ConsoleKit daemon is not running or if the caller doesn't have
+ * sufficient privileges.
+ *
+ * @returns #TRUE if the operation succeeds
+ */
 dbus_bool_t
-ckc_create_local_session2 (CKConnector *ckc)
+ck_connector_open_session (CkConnector *connector,
+                           DBusError   *out_error)
 {
-	DBusError error;
-	DBusMessage *message;
-	DBusMessage *reply;
-	dbus_bool_t ret;
-	char *cookie;
-	
-	reply = NULL;
-	message = NULL;
-	ret = FALSE;
-	
-	dbus_error_init (&error);
-	ckc->con = dbus_bus_get_private (DBUS_BUS_SYSTEM, &error);
-	if (ckc->con == NULL) {
-		goto out;
-	}
-	dbus_connection_set_exit_on_disconnect (ckc->con, FALSE);
-	
-	message = dbus_message_new_method_call ("org.freedesktop.ConsoleKit", 
-						"/org/freedesktop/ConsoleKit/Manager",
-						"org.freedesktop.ConsoleKit.Manager",
-						"OpenSession");
-	if (message == NULL)
-		goto out;
-	
-	reply = dbus_connection_send_with_reply_and_block (ckc->con, message, -1, &error);
-	if (reply == NULL) {
-		if (dbus_error_is_set (&error)) {
-			dbus_error_free (&error);
-			goto out;
-		}
-	}
-
-	if (!dbus_message_get_args (reply, 
-				    &error,
-				    DBUS_TYPE_STRING, &cookie,
-				    DBUS_TYPE_INVALID)) {
-		if (dbus_error_is_set (&error)) {
-			dbus_error_free (&error);
-			goto out;
-		}
-	}
-
-	ckc->cookie = strdup (cookie);
-	if (ckc->cookie == NULL)
-		goto out;
-
-	ckc->session_created = TRUE;
-	ret = TRUE;
-	
+        DBusError    error;
+        DBusMessage *message;
+        DBusMessage *reply;
+        dbus_bool_t  ret;
+        char        *cookie;
+
+        _ck_return_val_if_fail (connector != NULL, FALSE);
+
+        reply = NULL;
+        message = NULL;
+        ret = FALSE;
+
+        dbus_error_init (&error);
+        connector->connection = dbus_bus_get_private (DBUS_BUS_SYSTEM, &error);
+        if (connector->connection == NULL) {
+                if (dbus_error_is_set (&error)) {
+                        dbus_set_error (out_error,
+                                        CK_CONNECTOR_ERROR,
+                                        "Unable to open session: %s",
+                                        error.message);
+                        dbus_error_free (&error);
+                }
+
+                goto out;
+        }
+
+        dbus_connection_set_exit_on_disconnect (connector->connection, FALSE);
+
+        message = dbus_message_new_method_call ("org.freedesktop.ConsoleKit",
+                                                "/org/freedesktop/ConsoleKit/Manager",
+                                                "org.freedesktop.ConsoleKit.Manager",
+                                                "OpenSession");
+        if (message == NULL) {
+                goto out;
+        }
+
+        reply = dbus_connection_send_with_reply_and_block (connector->connection,
+                                                           message,
+                                                           -1,
+                                                           &error);
+        if (reply == NULL) {
+                if (dbus_error_is_set (&error)) {
+                        dbus_set_error (out_error,
+                                        CK_CONNECTOR_ERROR,
+                                        "Unable to open session: %s",
+                                        error.message);
+                        dbus_error_free (&error);
+                        goto out;
+                }
+        }
+
+        if (! dbus_message_get_args (reply,
+                                     &error,
+                                     DBUS_TYPE_STRING, &cookie,
+                                     DBUS_TYPE_INVALID)) {
+                if (dbus_error_is_set (&error)) {
+                        dbus_set_error (out_error,
+                                        CK_CONNECTOR_ERROR,
+                                        "Unable to open session: %s",
+                                        error.message);
+                        dbus_error_free (&error);
+                        goto out;
+                }
+        }
+
+        connector->cookie = strdup (cookie);
+        if (connector->cookie == NULL) {
+                goto out;
+        }
+
+        connector->session_created = TRUE;
+        ret = TRUE;
+
 out:
-	if (reply != NULL)
-		dbus_message_unref (reply);
+        if (reply != NULL) {
+                dbus_message_unref (reply);
+        }
+
+        if (message != NULL) {
+                dbus_message_unref (message);
+        }
 
-	if (message != NULL)
-		dbus_message_unref (message);
-	
-	return ret;
+        return ret;
 }
 
+/**
+ * Connects to the D-Bus system bus daemon and issues the method call
+ * OpenSessionWithParameters on the ConsoleKit manager interface. The
+ * connection to the bus is private.
+ *
+ * The only parameter that is optional is x11_display - it may be set
+ * to NULL if there is no X11 server associated with the session.
+ *
+ * Returns FALSE on OOM, if the system bus daemon is not running, if
+ * the ConsoleKit daemon is not running or if the caller doesn't have
+ * sufficient privileges.
+ *
+ * @param user UID for the user owning the session
+ * @param tty the tty device for the session
+ * @param x11_display the value of the X11 DISPLAY for the session
+ * @returns #TRUE if the operation succeeds
+ */
 dbus_bool_t
-ckc_create_local_session (CKConnector *ckc, uid_t user, const char *tty, const char *x11_display)
+ck_connector_open_session_for_user (CkConnector *connector,
+                                    uid_t        user,
+                                    const char  *tty,
+                                    const char  *x11_display,
+                                    DBusError   *out_error)
 {
-	DBusError error;
-	DBusMessage *message;
-	DBusMessage *reply;
-	DBusMessageIter iter;
-	DBusMessageIter iter_array;
-	dbus_bool_t ret;
-	char *cookie;
-	
-	reply = NULL;
-	message = NULL;
-	ret = FALSE;
-	
-	dbus_error_init (&error);
-	ckc->con = dbus_bus_get_private (DBUS_BUS_SYSTEM, &error);
-	if (ckc->con == NULL) {
-		goto out;
-	}
-	dbus_connection_set_exit_on_disconnect (ckc->con, FALSE);
-	
-	message = dbus_message_new_method_call ("org.freedesktop.ConsoleKit", 
-						"/org/freedesktop/ConsoleKit/Manager",
-						"org.freedesktop.ConsoleKit.Manager",
-						"OpenSessionWithParameters");
-	if (message == NULL)
-		goto out;
-	
-	dbus_message_iter_init_append (message, &iter);
-	if (!dbus_message_iter_open_container (&iter, 
-					       DBUS_TYPE_ARRAY,
-					       "(sv)", 
-					       &iter_array))
-		goto out;
-	
-	if (!add_param_string (&iter_array, "host-name", "localhost"))
-		goto out;
-	if (!add_param_string (&iter_array, "display-device", tty))
-		goto out;
-	if (x11_display != NULL)
-		if (!add_param_string (&iter_array, "x11-display", x11_display))
-			goto out;
-	if (!add_param_int32 (&iter_array, "user", user))
-		goto out;
-	if (!add_param_bool (&iter_array, "is-local", TRUE))
-		goto out;
-	
-	if (!dbus_message_iter_close_container (&iter, &iter_array))
-		goto out;
-	
-	reply = dbus_connection_send_with_reply_and_block (ckc->con, message, -1, &error);
-	if (reply == NULL) {
-		if (dbus_error_is_set (&error)) {
-			dbus_error_free (&error);
-			goto out;
-		}
-	}
-
-	if (!dbus_message_get_args (reply, 
-				    &error,
-				    DBUS_TYPE_STRING, &cookie,
-				    DBUS_TYPE_INVALID)) {
-		if (dbus_error_is_set (&error)) {
-			dbus_error_free (&error);
-			goto out;
-		}
-	}
-
-	ckc->cookie = strdup (cookie);
-	if (ckc->cookie == NULL)
-		goto out;
-
-	ckc->session_created = TRUE;
-	ret = TRUE;
-	
+        DBusError       error;
+        DBusMessage    *message;
+        DBusMessage    *reply;
+        DBusMessageIter iter;
+        DBusMessageIter iter_array;
+        dbus_bool_t     ret;
+        char           *cookie;
+
+        _ck_return_val_if_fail (connector != NULL, FALSE);
+        _ck_return_val_if_fail (user > 0, FALSE);
+        _ck_return_val_if_fail (tty != NULL, FALSE);
+
+        reply = NULL;
+        message = NULL;
+        ret = FALSE;
+
+        dbus_error_init (&error);
+        connector->connection = dbus_bus_get_private (DBUS_BUS_SYSTEM, &error);
+        if (connector->connection == NULL) {
+                if (dbus_error_is_set (&error)) {
+                        dbus_set_error (out_error,
+                                        CK_CONNECTOR_ERROR,
+                                        "Unable to open session: %s",
+                                        error.message);
+                        dbus_error_free (&error);
+                }
+                goto out;
+        }
+
+        dbus_connection_set_exit_on_disconnect (connector->connection, FALSE);
+
+        message = dbus_message_new_method_call ("org.freedesktop.ConsoleKit",
+                                                "/org/freedesktop/ConsoleKit/Manager",
+                                                "org.freedesktop.ConsoleKit.Manager",
+                                                "OpenSessionWithParameters");
+        if (message == NULL) {
+                goto out;
+        }
+
+        dbus_message_iter_init_append (message, &iter);
+        if (! dbus_message_iter_open_container (&iter,
+                                                DBUS_TYPE_ARRAY,
+                                                "(sv)",
+                                                &iter_array)) {
+                goto out;
+        }
+
+        if (! add_param_string (&iter_array, "host-name", "localhost")) {
+                goto out;
+        }
+
+        if (! add_param_string (&iter_array, "display-device", tty)) {
+                goto out;
+        }
+
+        if (x11_display != NULL) {
+                if (! add_param_string (&iter_array, "x11-display", x11_display)) {
+                        goto out;
+                }
+        }
+
+        if (! add_param_int32 (&iter_array, "user", user)) {
+                goto out;
+        }
+
+        if (! add_param_bool (&iter_array, "is-local", TRUE)) {
+                goto out;
+        }
+
+        if (! dbus_message_iter_close_container (&iter, &iter_array)) {
+                goto out;
+        }
+
+        reply = dbus_connection_send_with_reply_and_block (connector->connection,
+                                                           message,
+                                                           -1,
+                                                           &error);
+        if (reply == NULL) {
+                if (dbus_error_is_set (&error)) {
+                        dbus_set_error (out_error,
+                                        CK_CONNECTOR_ERROR,
+                                        "Unable to open session: %s",
+                                        error.message);
+                        dbus_error_free (&error);
+                        goto out;
+                }
+        }
+
+        if (! dbus_message_get_args (reply,
+                                     &error,
+                                     DBUS_TYPE_STRING, &cookie,
+                                     DBUS_TYPE_INVALID)) {
+                if (dbus_error_is_set (&error)) {
+                        dbus_set_error (out_error,
+                                        CK_CONNECTOR_ERROR,
+                                        "Unable to open session: %s",
+                                        error.message);
+                        dbus_error_free (&error);
+                        goto out;
+                }
+        }
+
+        connector->cookie = strdup (cookie);
+        if (connector->cookie == NULL) {
+                goto out;
+        }
+
+        connector->session_created = TRUE;
+        ret = TRUE;
+
 out:
-	if (reply != NULL)
-		dbus_message_unref (reply);
+        if (reply != NULL) {
+                dbus_message_unref (reply);
+        }
 
-	if (message != NULL)
-		dbus_message_unref (message);
-	
-	return ret;
+        if (message != NULL) {
+                dbus_message_unref (message);
+        }
+
+        return ret;
 }
 
-char *
-ckc_get_cookie (CKConnector *ckc)
+/**
+ * Gets the cookie for the current open session.
+ * Returns #NULL if no session is open.
+ *
+ * @returns a constant string with the cookie.
+ */
+const char *
+ck_connector_get_cookie (CkConnector *connector)
 {
-	if (!ckc->session_created)
-		return NULL;
-	else
-		return ckc->cookie;
+        _ck_return_val_if_fail (connector != NULL, NULL);
+
+        if (! connector->session_created) {
+                return NULL;
+        } else {
+                return connector->cookie;
+        }
 }
 
+/**
+ * Issues the CloseSession method call on the ConsoleKit manager
+ * interface.
+ *
+ * Returns FALSE on OOM, if the system bus daemon is not running, if
+ * the ConsoleKit daemon is not running, if the caller doesn't have
+ * sufficient privilege or if a session isn't open.
+ *
+ * @returns #TRUE if the operation succeeds
+ */
 dbus_bool_t
-ckc_close_session (CKConnector *ckc)
+ck_connector_close_session (CkConnector *connector,
+                            DBusError   *out_error)
 {
-	DBusError error;
-	DBusMessage *message;
-	DBusMessage *reply;
-	dbus_bool_t ret;
-	dbus_bool_t session_closed;
-	
-	reply = NULL;
-	message = NULL;
-	ret = FALSE;
-	if (!ckc->session_created || ckc->cookie == NULL)
-		goto out;
-	
-	dbus_error_init (&error);
-	message = dbus_message_new_method_call ("org.freedesktop.ConsoleKit", 
-						"/org/freedesktop/ConsoleKit/Manager",
-						"org.freedesktop.ConsoleKit.Manager",
-						"CloseSession");
-	if (message == NULL)
-		goto out;
-
-	if (!dbus_message_append_args (message, 
-				       DBUS_TYPE_STRING, &(ckc->cookie),
-				       DBUS_TYPE_INVALID))
-		goto out;
-	
-	reply = dbus_connection_send_with_reply_and_block (ckc->con, message, -1, &error);
-	if (reply == NULL) {
-		if (dbus_error_is_set (&error)) {
-			dbus_error_free (&error);
-			goto out;
-		}
-	}
-
-	if (!dbus_message_get_args (reply, 
-				    &error,
-				    DBUS_TYPE_BOOLEAN, &session_closed,
-				    DBUS_TYPE_INVALID)) {
-		if (dbus_error_is_set (&error)) {
-			dbus_error_free (&error);
-			goto out;
-		}
-	}
-
-	if (!session_closed)
-		goto out;
-
-	ckc->session_created = FALSE;
-	ret = TRUE;
-	
+        DBusError    error;
+        DBusMessage *message;
+        DBusMessage *reply;
+        dbus_bool_t  ret;
+        dbus_bool_t  session_closed;
+
+        reply = NULL;
+        message = NULL;
+        ret = FALSE;
+
+        if (!connector->session_created || connector->cookie == NULL) {
+                dbus_set_error (out_error,
+                                CK_CONNECTOR_ERROR,
+                                "Unable to close session: %s",
+                                "no session open");
+                goto out;
+        }
+
+        dbus_error_init (&error);
+        message = dbus_message_new_method_call ("org.freedesktop.ConsoleKit",
+                                                "/org/freedesktop/ConsoleKit/Manager",
+                                                "org.freedesktop.ConsoleKit.Manager",
+                                                "CloseSession");
+        if (message == NULL) {
+                goto out;
+        }
+
+        if (! dbus_message_append_args (message,
+                                        DBUS_TYPE_STRING, &(connector->cookie),
+                                        DBUS_TYPE_INVALID)) {
+                goto out;
+        }
+
+        reply = dbus_connection_send_with_reply_and_block (connector->connection,
+                                                           message,
+                                                           -1,
+                                                           &error);
+        if (reply == NULL) {
+                if (dbus_error_is_set (&error)) {
+                        dbus_set_error (out_error,
+                                        CK_CONNECTOR_ERROR,
+                                        "Unable to close session: %s",
+                                        error.message);
+                        dbus_error_free (&error);
+                        goto out;
+                }
+        }
+
+        if (! dbus_message_get_args (reply,
+                                     &error,
+                                     DBUS_TYPE_BOOLEAN, &session_closed,
+                                     DBUS_TYPE_INVALID)) {
+                if (dbus_error_is_set (&error)) {
+                        dbus_set_error (out_error,
+                                        CK_CONNECTOR_ERROR,
+                                        "Unable to close session: %s",
+                                        error.message);
+                        dbus_error_free (&error);
+                        goto out;
+                }
+        }
+
+        if (! session_closed) {
+                goto out;
+        }
+
+        connector->session_created = FALSE;
+        ret = TRUE;
+
 out:
-	if (reply != NULL)
-		dbus_message_unref (reply);
+        if (reply != NULL) {
+                dbus_message_unref (reply);
+        }
 
-	if (message != NULL)
-		dbus_message_unref (message);
-	
-	return ret;
+        if (message != NULL) {
+                dbus_message_unref (message);
+        }
 
-}
+        return ret;
 
-void
-ckc_free (CKConnector *ckc)
-{
-	if (ckc->con != NULL) {
-		/* it's a private connection so it's all good */
-		dbus_connection_close (ckc->con);
-	}
-	if (ckc->cookie != NULL) {
-		free (ckc->cookie);
-	}
-	free (ckc);
 }
diff --git a/libck-connector/ck-connector.h b/libck-connector/ck-connector.h
index e92712d..2ea1bbb 100644
--- a/libck-connector/ck-connector.h
+++ b/libck-connector/ck-connector.h
@@ -1,8 +1,10 @@
-/*
+/* -*- Mode: C; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 8 -*-
+ *
  * ck-connector.h : Code for login managers to register with ConsoleKit.
  *
  * Copyright (c) 2007 David Zeuthen <davidz@redhat.com>
- * 
+ * Copyright (c) 2007 William Jon McCann <mccann@jhu.edu>
+ *
  * Permission is hereby granted, free of charge, to any person
  * obtaining a copy of this software and associated documentation
  * files (the "Software"), to deal in the Software without
@@ -11,10 +13,10 @@
  * copies of the Software, and to permit persons to whom the
  * Software is furnished to do so, subject to the following
  * conditions:
- * 
+ *
  * The above copyright notice and this permission notice shall be
  * included in all copies or substantial portions of the Software.
- * 
+ *
  * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
  * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
  * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
@@ -31,92 +33,28 @@
 #include <sys/types.h>
 #include <dbus/dbus.h>
 
-struct CKConnecter_s;
-typedef struct CKConnector_s CKConnector;
+DBUS_BEGIN_DECLS
 
-/* Allocates a new CKConnector instance.
- *
- * Returns NULL on OOM */
-CKConnector  *ckc_new                  (void);
+struct _CkConnecter;
+typedef struct _CkConnector CkConnector;
 
-/* Connects to the D-Bus system bus daemon and issues the method call
- * OpenSessionWithParameters on the ConsoleKit manager interface. The
- * connection to the bus is private. 
- *
- * The only parameter that is optional is x11_display - it may be set
- * to NULL if there is no X11 server associated with the session.
- *
- * Returns FALSE on OOM, if the system bus daemon is not running, if
- * the ConsoleKit daemon is not running or if the caller doesn't have
- * sufficient privileges.
- */
-dbus_bool_t   ckc_create_local_session (CKConnector *ckc, uid_t user, const char *tty, const char *x11_display);
+CkConnector  *ck_connector_new                   (void);
 
-dbus_bool_t   ckc_create_local_session2 (CKConnector *ckc);
+CkConnector  *ck_connector_ref                   (CkConnector *ckc);
+void          ck_connector_unref                 (CkConnector *ckc);
 
-/* Gets the cookie that should be set as XDG_SESSION_COOKIE in the
- * users environment.
- *
- * Returns NULL unless ckc_create_local_session() succeeded 
- */
-char         *ckc_get_cookie           (CKConnector *ckc);
+dbus_bool_t   ck_connector_open_session_for_user (CkConnector *ckc,
+                                                  uid_t        user,
+                                                  const char  *tty,
+                                                  const char  *x11_display,
+                                                  DBusError   *error);
+dbus_bool_t   ck_connector_open_session          (CkConnector *ckc,
+                                                  DBusError   *error);
 
-/* Issues the CloseSession method call on the ConsoleKit manager
- * interface.
- *
- * Returns FALSE on OOM, if the system bus daemon is not running, if
- * the ConsoleKit daemon is not running, if the caller doesn't have
- * sufficient privilege or if ckc_create_local_session() wasn't
- * successfully invoked.
- */
-dbus_bool_t   ckc_close_session        (CKConnector *ckc);
+const char   *ck_connector_get_cookie            (CkConnector *ckc);
+dbus_bool_t   ck_connector_close_session         (CkConnector *ckc,
+                                                  DBusError   *error);
 
-/* Frees all resources allocated and disconnects from the system
- * message bus.
- */
-void          ckc_free                 (CKConnector *ckc);
-
-/* example code:
-
-#include <stdio.h>
-#include <stdlib.h>
-#include "ck-connector.h"
-
-int
-main (int argc, char *argv[])
-{
-	CKConnector *ckc;
-
-	ckc = ckc_new ();
-	if (ckc == NULL) {
-		printf ("OOM creating CKConnector\n");
-		goto out;
-	}
-
-	if (!ckc_create_local_session (ckc, 500, "/dev/tty2", ":1")) {
-		printf ("cannot create CK session: OOM, D-Bus system bus not available,\n"
-			"ConsoleKit not available or insufficient privileges.\n");
-		goto out;
-	}
-
-	printf ("Session cookie is '%s'\n", ckc_get_cookie (ckc));
-	sleep (20);
-
-	if (!ckc_close_session (ckc)) {
-		printf ("Cannot close CK session: OOM, D-Bus system bus not available,\n"
-			"ConsoleKit not available or insufficient privileges.\n");
-		goto out;
-	}
-
-out:
-	if (ckc != NULL) {
-		ckc_free (ckc);
-	}
-}
-
-*/
+DBUS_END_DECLS
 
 #endif /* CK_CONNECTOR_H */
-
-
-
diff --git a/pam-ck-connector/pam-ck-connector.c b/pam-ck-connector/pam-ck-connector.c
index 835be0c..c026806 100644
--- a/pam-ck-connector/pam-ck-connector.c
+++ b/pam-ck-connector/pam-ck-connector.c
@@ -1,8 +1,9 @@
-/*
+/* -*- Mode: C; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 8 -*-
+ *
  * pam-ck-connector.c : PAM module for registering with CK
  *
  * Copyright (c) 2007 David Zeuthen <davidz@redhat.com>
- * 
+ *
  * Permission is hereby granted, free of charge, to any person
  * obtaining a copy of this software and associated documentation
  * files (the "Software"), to deal in the Software without
@@ -11,10 +12,10 @@
  * copies of the Software, and to permit persons to whom the
  * Software is furnished to do so, subject to the following
  * conditions:
- * 
+ *
  * The above copyright notice and this permission notice shall be
  * included in all copies or substantial portions of the Software.
- * 
+ *
  * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
  * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
  * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
@@ -50,40 +51,50 @@
 static int opt_debug = FALSE;
 
 static void
-_parse_pam_args (const pam_handle_t *pamh, int flags, int argc, const char **argv)
+_parse_pam_args (const pam_handle_t *pamh,
+                 int                 flags,
+                 int                 argc,
+                 const char        **argv)
 {
-	int i;
-
-	opt_debug = FALSE;
-	for (i = 0; i < argc && argv[i] != NULL; i++) {
-		if (strcmp (argv[i] ,"debug") == 0) {
-			opt_debug = TRUE;
-		} else {
-			pam_syslog (pamh, LOG_ERR, "unknown option: %s", argv[i]);
-		}
-	}
+        int i;
+
+        opt_debug = FALSE;
+        for (i = 0; i < argc && argv[i] != NULL; i++) {
+                if (strcmp (argv[i] ,"debug") == 0) {
+                        opt_debug = TRUE;
+                } else {
+                        pam_syslog (pamh, LOG_ERR, "unknown option: %s", argv[i]);
+                }
+        }
 }
 
 
 PAM_EXTERN int
-pam_sm_authenticate (pam_handle_t *pamh, int flags, int argc, const char **argv)
+pam_sm_authenticate (pam_handle_t *pamh,
+                     int           flags,
+                     int           argc,
+                     const char  **argv)
 {
-	return PAM_IGNORE;
+        return PAM_IGNORE;
 }
 
-PAM_EXTERN int 
-pam_sm_setcred (pam_handle_t *pamh, int flags, int argc, const char **argv)
+PAM_EXTERN int
+pam_sm_setcred (pam_handle_t *pamh,
+                int           flags,
+                int           argc,
+                const char  **argv)
 {
-	return PAM_IGNORE;
+        return PAM_IGNORE;
 }
 
 static uid_t
-_util_name_to_uid (const char *username, gid_t *default_gid)
+_util_name_to_uid (const char *username,
+                   gid_t      *default_gid)
 {
-        int rc;
-        uid_t res;
-        char *buf = NULL;
-        unsigned int bufsize;
+        int           rc;
+        uid_t         res;
+        char         *buf = NULL;
+        unsigned int  bufsize;
         struct passwd pwd;
         struct passwd *pwdp;
 
@@ -97,8 +108,9 @@ _util_name_to_uid (const char *username, gid_t *default_gid)
         }
 
         res = pwdp->pw_uid;
-        if (default_gid != NULL)
+        if (default_gid != NULL) {
                 *default_gid = pwdp->pw_gid;
+        }
 
 out:
         free (buf);
@@ -108,119 +120,138 @@ out:
 /* our singleton */
 static CKConnector *ckc = NULL;
 
-PAM_EXTERN int 
-pam_sm_close_session (pam_handle_t *pamh, int flags, int argc, const char **argv)
+PAM_EXTERN int
+pam_sm_close_session (pam_handle_t *pamh,
+                      int           flags,
+                      int           argc,
+                      const char  **argv)
 {
-	if (ckc != NULL) {
-		ckc_free (ckc);
-		ckc = NULL;
-	}
-	return PAM_SUCCESS;
+        if (ckc != NULL) {
+                ck_connector_unref (ckc);
+                ckc = NULL;
+        }
+        return PAM_SUCCESS;
 }
 
-PAM_EXTERN int 
-pam_sm_open_session (pam_handle_t *pamh, int flags, int argc, const char **argv)
+PAM_EXTERN int
+pam_sm_open_session (pam_handle_t *pamh,
+                     int           flags,
+                     int           argc,
+                     const char  **argv)
 {
-	int ret;
-	const char *user;
-	const char *tty;
-	const char *x11_display;
-	const char *s;
-	uid_t uid;
-	char buf[256];
-
-	ret = PAM_IGNORE;
-
-	_parse_pam_args (pamh, flags, argc, argv);
-
-	/* Register with ConsoleKit as part of the session management */
-	if (ckc != NULL) {
-		pam_syslog (pamh, LOG_ERR, "process already registered with ConsoleKit");
-		goto out;
-	}
-
-	ckc = ckc_new ();
-	if (ckc == NULL) {
-		pam_syslog (pamh, LOG_ERR, "oom creating ConsoleKit connector object");
-		goto out;
-	}
-
-	if (pam_get_user (pamh, &user, NULL) != PAM_SUCCESS || user == NULL) {
-		pam_syslog (pamh, LOG_ERR, "cannot determine username");
-		goto out;
-	}
-
-	if (pam_get_item (pamh, PAM_TTY, (const void **) &tty) != PAM_SUCCESS || tty == NULL) {
-		pam_syslog (pamh, LOG_ERR, "cannot determine tty");
-		goto out;
-	}
-
-	if ((s = pam_getenv (pamh, "CKCON_TTY")) != NULL) {
-		tty = s;
-		if (opt_debug)
-			pam_syslog (pamh, LOG_INFO, "using '%s' as tty (from CKCON_TTY)", tty);
-	}
-
-	x11_display = NULL;
-	if ((s = pam_getenv (pamh, "CKCON_X11_DISPLAY")) != NULL) {
-		x11_display = s;
-		if (opt_debug)
-			pam_syslog (pamh, LOG_INFO, "using '%s' as X11 display (from CKCON_X11_DISPLAY)", x11_display);
-	}
-
-	uid = _util_name_to_uid (user, NULL);
-	if (uid == (uid_t) -1) {
-		pam_syslog (pamh, LOG_ERR, "cannot determine uid for user '%s'", user);
-		goto out;
-	}
-
-	if (!ckc_create_local_session (ckc, uid, tty, x11_display)) {
-		/* this might not be a bug for servers that don't have
-		 * the message bus or ConsoleKit daemon running - so
-		 * only log a message in debugging mode.
-		 */
-		if (opt_debug)
-			pam_syslog (pamh, LOG_DEBUG, "insufficient privileges or D-Bus / ConsoleKit not available");
-		goto out;
-	}
-
-	/* now set the cookie */
-	buf[sizeof (buf) - 1] = '\0';
-	snprintf (buf, sizeof (buf) - 1, "XDG_SESSION_COOKIE=%s", ckc_get_cookie (ckc));
-	if (pam_putenv (pamh, buf) != PAM_SUCCESS) {
-		pam_syslog (pamh, LOG_ERR, "unable to set XDG_SESSION_COOKIE vairable");
-		/* tear down session the hard way */
-		ckc_free (ckc);
-		ckc = NULL;
-		goto out;
-	}
-		    
-	if (opt_debug) {
-		pam_syslog (pamh, LOG_DEBUG, "registered uid=%d on tty='%s' with ConsoleKit", uid, tty);
-	}
-
-	/* note that we're leaking our CKConnector instance ckc - this
-	 * is *by design* such that when the login manager (that uses
-	 * us) exits / crashes / etc. ConsoleKit will notice, via D-Bus
-	 * connection tracking, that the login session ended.
-	 */
-
-	ret = PAM_SUCCESS;
+        int         ret;
+        const char *user;
+        const char *tty;
+        const char *x11_display;
+        const char *s;
+        uid_t       uid;
+        char        buf[256];
+        DBusError   error;
+
+        ret = PAM_IGNORE;
+
+        _parse_pam_args (pamh, flags, argc, argv);
+
+        /* Register with ConsoleKit as part of the session management */
+        if (ckc != NULL) {
+                pam_syslog (pamh, LOG_ERR, "process already registered with ConsoleKit");
+                goto out;
+        }
+
+        ckc = ck_connector_new ();
+        if (ckc == NULL) {
+                pam_syslog (pamh, LOG_ERR, "oom creating ConsoleKit connector object");
+                goto out;
+        }
+
+        if (pam_get_user (pamh, &user, NULL) != PAM_SUCCESS || user == NULL) {
+                pam_syslog (pamh, LOG_ERR, "cannot determine username");
+                goto out;
+        }
+
+        if (pam_get_item (pamh, PAM_TTY, (const void **) &tty) != PAM_SUCCESS || tty == NULL) {
+                pam_syslog (pamh, LOG_ERR, "cannot determine tty");
+                goto out;
+        }
+
+        if ((s = pam_getenv (pamh, "CKCON_TTY")) != NULL) {
+                tty = s;
+                if (opt_debug) {
+                        pam_syslog (pamh, LOG_INFO, "using '%s' as tty (from CKCON_TTY)", tty);
+                }
+        }
+
+        x11_display = NULL;
+        if ((s = pam_getenv (pamh, "CKCON_X11_DISPLAY")) != NULL) {
+                x11_display = s;
+                if (opt_debug)
+                        pam_syslog (pamh, LOG_INFO, "using '%s' as X11 display (from CKCON_X11_DISPLAY)", x11_display);
+        }
+
+        uid = _util_name_to_uid (user, NULL);
+        if (uid == (uid_t) -1) {
+                pam_syslog (pamh, LOG_ERR, "cannot determine uid for user '%s'", user);
+                goto out;
+        }
+
+        dbus_error_init (&error);
+        if (! ck_connector_open_session_for_user (ckc, uid, tty, x11_display, &error)) {
+                /* this might not be a bug for servers that don't have
+                 * the message bus or ConsoleKit daemon running - so
+                 * only log a message in debugging mode.
+                 */
+                if (dbus_error_is_set (&error)) {
+                        if (opt_debug) {
+                                pam_syslog (pamh, LOG_DEBUG, "%s", error.message);
+                        }
+                        dbus_error_free (&error);
+                } else {
+                        if (opt_debug) {
+                                pam_syslog (pamh, LOG_DEBUG, "insufficient privileges or
+ D-Bus / ConsoleKit not available");
+                        }
+                }
+
+                goto out;
+        }
+
+        /* now set the cookie */
+        buf[sizeof (buf) - 1] = '\0';
+        snprintf (buf, sizeof (buf) - 1, "XDG_SESSION_COOKIE=%s", ck_connector_get_cookie (ckc));
+        if (pam_putenv (pamh, buf) != PAM_SUCCESS) {
+                pam_syslog (pamh, LOG_ERR, "unable to set XDG_SESSION_COOKIE vairable");
+                /* tear down session the hard way */
+                ck_connector_unref (ckc);
+                ckc = NULL;
+                goto out;
+        }
+
+        if (opt_debug) {
+                pam_syslog (pamh, LOG_DEBUG, "registered uid=%d on tty='%s' with ConsoleKit", uid, tty);
+        }
+
+        /* note that we're leaking our CKConnector instance ckc - this
+         * is *by design* such that when the login manager (that uses
+         * us) exits / crashes / etc. ConsoleKit will notice, via D-Bus
+         * connection tracking, that the login session ended.
+         */
+
+        ret = PAM_SUCCESS;
 
 out:
-	return ret;
+        return ret;
 }
 
 #ifdef PAM_STATIC
 
 struct pam_module _pam_ckconnector_modstruct = {
-	"pam_ck_connector",
-	pam_sm_authenticate,
-	pam_sm_setcred,
-	NULL,
-	pam_sm_open_session,
-	pam_sm_close_session,
-	NULL,
+        "pam_ck_connector",
+        pam_sm_authenticate,
+        pam_sm_setcred,
+        NULL,
+        pam_sm_open_session,
+        pam_sm_close_session,
+        NULL,
 };
 
 #endif