sbus: Add struct sbus_request to represent a DBus invocation

struct sbus_request represents a request from a dbus client
being handled by a dbus server implementation. The struct
contains the message, connection and method (and in the
future teh property) which is being requested.

In the future it will contain caller information as well.

sbus_request is a talloc memory context, and is a good place to
attach any allocations and memory specific to the request.

Each handler accepts an sbus_request. If a handler returns
EOK, it is assumed that the handler will finish the request.
Any of the sbus_request_*finish() methods can be used to
complete the request and send back a reply.

sbus_request_return_and_finish() uses the same argument
varargs syntax as dbus_message_append_args(), which isn't
a great syntax. Document it a bit, but don't try to redesign:
The marshalling work (will follow this patch set) will remove
the need to use varargs for most DBus implementation code.

This patch migrates the monitor and data provider dbus code
to use sbus_request, but does not try to rework the talloc
context's to use it.

Reviewed-by: Jakub Hrozek <jhrozek@redhat.com>
Reviewed-by: Pavel Březina <pbrezina@redhat.com>
Reviewed-by: Lukáš Slebodník <lslebodn@redhat.com>

4 files changed, 221 insertions, 21 deletions

diff --git a/src/sbus/sssd_dbus.h b/src/sbus/sssd_dbus.h
index 7d00b94d0..dcb669079 100644
--- a/src/sbus/sssd_dbus.h
+++ b/src/sbus/sssd_dbus.h
@@ -24,9 +24,11 @@
 
 struct sbus_connection;
 
+struct sbus_request;
+
 #include <dbus/dbus.h>
 
-typedef int (*sbus_msg_handler_fn)(DBusMessage *, struct sbus_connection *);
+typedef int (*sbus_msg_handler_fn)(struct sbus_request *dbus_req);
 
 /*
  * sbus_conn_destructor_fn
@@ -170,4 +172,78 @@ int sbus_conn_send(struct sbus_connection *conn,
 void sbus_conn_send_reply(struct sbus_connection *conn,
                           DBusMessage *reply);
 
+/*
+ * This structure is passed to all dbus method and property
+ * handlers. It is a talloc context which will be valid until
+ * the request is completed with either the sbus_request_complete()
+ * or sbus_request_fail() functions.
+ */
+struct sbus_request {
+    struct sbus_connection *conn;
+    DBusMessage *message;
+    struct sbus_interface *intf;
+    const struct sbus_method_meta *method;
+};
+
+/*
+ * Complete a DBus request, and free the @dbus_req context. The @dbus_req
+ * and associated talloc context are no longer valid after this function
+ * returns.
+ *
+ * If @reply is non-NULL then the reply is sent to the caller. Not sending
+ * a reply when the caller is expecting one is fairly rude behavior.
+ *
+ * The return value is useful for logging, but not much else. In particular
+ * even if this function return !EOK, @dbus_req is still unusable after this
+ * function returns.
+ */
+int sbus_request_finish(struct sbus_request *dbus_req,
+                        DBusMessage *reply);
+
+/*
+ * Return a reply for a DBus method call request. The variable
+ * arguments are (unfortunately) formatted exactly the same as those of the
+ * dbus_message_append_args() function. Documented here:
+ *
+ * http://dbus.freedesktop.org/doc/api/html/group__DBusMessage.html
+ *
+ * Important: don't pass int or bool or such types as
+ * values to this function. That's not portable. Use actual dbus types.
+ * You must also pass pointers as the values:
+ *
+ *    dbus_bool_t val1 = TRUE;
+ *    dbus_int32_t val2 = 5;
+ *    ret = sbus_request_finish(dbus_req,
+ *                              DBUS_TYPE_BOOLEAN, &val1,
+ *                              DBUS_TYPE_INT32, &val2,
+ *                              DBUS_TYPE_INVALID);
+ *
+ * To pass arrays to this function, use the following syntax. Never
+ * pass actual C arrays with [] syntax to this function. The C standard is
+ * rather vague with C arrays and varargs, and it just plain doesn't work.
+ *
+ *    const char *array[] = { "one", "two", "three" };
+ *    int count = 3; // yes, a plain int
+ *    const char **ptr = array;
+ *    ret = sbus_request_finish(dbus_req,
+ *                              DBUS_TYPE_ARRAY, DBUS_TYPE_STRING, &ptr, 3,
+ *                              DBUS_TYPE_INVALID);
+ *
+ * The @dbus_req and associated talloc context are no longer valid after this
+ * function returns, even if this function returns an error code.
+ */
+int sbus_request_return_and_finish(struct sbus_request *dbus_req,
+                                   int first_arg_type,
+                                   ...);
+
+/*
+ * Return an error for a DBus method call request. The @error is a normal
+ * DBusError.
+ *
+ * The @dbus_req and associated talloc context are no longer valid after this
+ * function returns, even if this function returns an error code.
+ */
+int sbus_request_fail_and_finish(struct sbus_request *dbus_req,
+                                 const DBusError *error);
+
 #endif /* _SSSD_DBUS_H_*/
diff --git a/src/sbus/sssd_dbus_connection.c b/src/sbus/sssd_dbus_connection.c
index d39f1c01f..5faf24949 100644
--- a/src/sbus/sssd_dbus_connection.c
+++ b/src/sbus/sssd_dbus_connection.c
@@ -416,7 +416,9 @@ DBusHandlerResult sbus_message_handler(DBusConnection *dbus_conn,
     DBusMessage *reply = NULL;
     const struct sbus_method_meta *method;
     const struct sbus_interface_meta *interface;
-    sbus_msg_handler_fn handler_fn;
+    struct sbus_request *dbus_req = NULL;
+    sbus_msg_handler_fn handler_fn = NULL;
+    DBusHandlerResult result;
     int ret;
 
     if (!user_data) {
@@ -436,10 +438,11 @@ DBusHandlerResult sbus_message_handler(DBusConnection *dbus_conn,
     if (strcmp(path, intf_p->intf->path) != 0)
         return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
 
+    result = DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
+
     /* Validate the method interface */
     interface = intf_p->intf->vtable->meta;
     if (strcmp(msg_interface, interface->name) == 0) {
-        handler_fn = NULL;
         method = sbus_meta_find_method(interface, msg_method);
         if (method && method->vtable_offset)
             handler_fn = VTABLE_FUNC(intf_p->intf->vtable, method->vtable_offset);
@@ -451,6 +454,7 @@ DBusHandlerResult sbus_message_handler(DBusConnection *dbus_conn,
             reply = dbus_message_new_error(message, DBUS_ERROR_UNKNOWN_METHOD, NULL);
             sbus_conn_send_reply(intf_p->conn, reply);
             dbus_message_unref(reply);
+            result = DBUS_HANDLER_RESULT_HANDLED;
 
         } else if (!handler_fn) {
             /* Reply DBUS_ERROR_NOT_SUPPORTED */
@@ -459,11 +463,7 @@ DBusHandlerResult sbus_message_handler(DBusConnection *dbus_conn,
             reply = dbus_message_new_error(message, DBUS_ERROR_NOT_SUPPORTED, NULL);
             sbus_conn_send_reply(intf_p->conn, reply);
             dbus_message_unref(reply);
-
-        } else {
-            ret = handler_fn(message, intf_p->conn);
-            if (ret != EOK)
-                return sbus_reply_internal_error(message, intf_p->conn);
+            result = DBUS_HANDLER_RESULT_HANDLED;
         }
     }
     else {
@@ -473,21 +473,28 @@ DBusHandlerResult sbus_message_handler(DBusConnection *dbus_conn,
         if (strcmp(msg_interface, DBUS_INTROSPECT_INTERFACE) == 0 &&
             strcmp(msg_method, DBUS_INTROSPECT_METHOD) == 0)
         {
-            if (intf_p->intf->introspect_fn) {
-                /* If we have been asked for introspection data and we have
-                 * an introspection function registered, user that.
-                 */
-                ret = intf_p->intf->introspect_fn(message, intf_p->conn);
-                if (ret != EOK) {
-                    return sbus_reply_internal_error(message, intf_p->conn);
-                }
-            }
+            handler_fn = intf_p->intf->introspect_fn;
+        }
+    }
+
+    if (handler_fn) {
+        dbus_req = sbus_new_request(intf_p->conn, intf_p->intf, message);
+        if (!dbus_req) {
+            ret = ENOMEM;
+        } else {
+            dbus_req->method = method;
+            ret = handler_fn(dbus_req);
+        }
+        if (ret != EOK) {
+            if (dbus_req)
+                talloc_free(dbus_req);
+            result = sbus_reply_internal_error(message, intf_p->conn);
+        } else {
+            result = DBUS_HANDLER_RESULT_HANDLED;
         }
-        else
-            return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
     }
 
-    return DBUS_HANDLER_RESULT_HANDLED;
+    return result;
 }
 
 /* Adds a new D-BUS path message handler to the connection
@@ -786,4 +793,3 @@ void sbus_conn_send_reply(struct sbus_connection *conn, DBusMessage *reply)
 {
     dbus_connection_send(conn->dbus.conn, reply, NULL);
 }
-
diff --git a/src/sbus/sssd_dbus_private.h b/src/sbus/sssd_dbus_private.h
index c62e0e3f6..eca5c8380 100644
--- a/src/sbus/sssd_dbus_private.h
+++ b/src/sbus/sssd_dbus_private.h
@@ -96,4 +96,10 @@ dbus_bool_t sbus_add_timeout(DBusTimeout *dbus_timeout, void *data);
 void sbus_toggle_timeout(DBusTimeout *dbus_timeout, void *data);
 void sbus_remove_timeout(DBusTimeout *dbus_timeout, void *data);
 
+/* =Requests============================================================== */
+
+struct sbus_request *
+sbus_new_request(struct sbus_connection *conn, struct sbus_interface *intf,
+                 DBusMessage *message);
+
 #endif /* _SSSD_DBUS_PRIVATE_H_ */
diff --git a/src/sbus/sssd_dbus_request.c b/src/sbus/sssd_dbus_request.c
new file mode 100644
index 000000000..973089c4b
--- /dev/null
+++ b/src/sbus/sssd_dbus_request.c
@@ -0,0 +1,112 @@
+/*
+    Authors:
+        Stef Walter <stefw@redhat.com>
+
+    Copyright (C) 2014 Red Hat
+
+    This program is free software; you can redistribute it and/or modify
+    it under the terms of the GNU General Public License as published by
+    the Free Software Foundation; either version 3 of the License, or
+    (at your option) any later version.
+
+    This program is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+    GNU General Public License for more details.
+
+    You should have received a copy of the GNU General Public License
+    along with this program.  If not, see <http://www.gnu.org/licenses/>.
+*/
+
+#include "util/util.h"
+#include "sbus/sssd_dbus.h"
+
+#include <sys/time.h>
+#include <dbus/dbus.h>
+
+static int sbus_request_destructor(struct sbus_request *dbus_req)
+{
+    dbus_message_unref(dbus_req->message);
+    return 0;
+}
+
+struct sbus_request *
+sbus_new_request(struct sbus_connection *conn,
+                 struct sbus_interface *intf,
+                 DBusMessage *message)
+{
+    struct sbus_request *dbus_req;
+
+    dbus_req = talloc_zero(conn, struct sbus_request);
+    if (!dbus_req) {
+        DEBUG(SSSDBG_CRIT_FAILURE, "Out of memory allocating DBus request\n");
+        return NULL;
+    }
+
+    dbus_req->intf = intf;
+    dbus_req->conn = conn;
+    dbus_req->message = dbus_message_ref(message);
+    talloc_set_destructor(dbus_req, sbus_request_destructor);
+
+    return dbus_req;
+}
+
+int sbus_request_finish(struct sbus_request *dbus_req,
+                        DBusMessage *reply)
+{
+    if (reply) {
+        sbus_conn_send_reply(dbus_req->conn, reply);
+    }
+    return talloc_free(dbus_req);
+}
+
+int sbus_request_return_and_finish(struct sbus_request *dbus_req,
+                                   int first_arg_type,
+                                   ...)
+{
+    DBusMessage *reply;
+    dbus_bool_t dbret;
+    va_list va;
+    int ret;
+
+    reply = dbus_message_new_method_return(dbus_req->message);
+    if (!reply) {
+        DEBUG(SSSDBG_CRIT_FAILURE, "Out of memory allocating DBus message\n");
+        sbus_request_finish(dbus_req, NULL);
+        return ENOMEM;
+    }
+
+    va_start(va, first_arg_type);
+    dbret = dbus_message_append_args_valist(reply, first_arg_type, va);
+    va_end(va);
+
+    if (dbret) {
+        ret = sbus_request_finish(dbus_req, reply);
+
+    } else {
+        DEBUG(SSSDBG_CRIT_FAILURE, "Couldn't build DBus message\n");
+        sbus_request_finish(dbus_req, NULL);
+        ret = EINVAL;
+    }
+
+    dbus_message_unref(reply);
+    return ret;
+}
+
+int sbus_request_fail_and_finish(struct sbus_request *dbus_req,
+                                 const DBusError *error)
+{
+    DBusMessage *reply;
+    int ret;
+
+    reply = dbus_message_new_error(dbus_req->message, error->name, error->message);
+    if (!reply) {
+        DEBUG(SSSDBG_CRIT_FAILURE, "Out of memory allocating DBus message\n");
+        sbus_request_finish(dbus_req, NULL);
+        return ENOMEM;
+    }
+
+    ret = sbus_request_finish(dbus_req, reply);
+    dbus_message_unref(reply);
+    return ret;
+}