2003-04-03 Havoc Pennington <hp@redhat.com>

	* bus/activation.c (load_directory): fix up memleaks
	(bus_activation_entry_free): free the entry

	* dbus/dbus-bus.c (dbus_bus_acquire_service): return an error if
	we get one from the message bus; fix memleaks.

	* dbus/dbus-message.c (dbus_set_error_from_message): new function

10 files changed, 201 insertions, 25 deletions

diff --git a/ChangeLog b/ChangeLog
index 3772119b..a164df07 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,13 @@
+2003-04-03  Havoc Pennington  <hp@redhat.com>
+
+	* bus/activation.c (load_directory): fix up memleaks
+	(bus_activation_entry_free): free the entry
+
+	* dbus/dbus-bus.c (dbus_bus_acquire_service): return an error if
+	we get one from the message bus; fix memleaks.
+
+	* dbus/dbus-message.c (dbus_set_error_from_message): new function
+
 2003-04-03  Havoc Pennington  <hp@pobox.com>
 
 	* bus/config-parser.c (bus_config_parser_unref): free 
diff --git a/bus/activation.c b/bus/activation.c
index eb56a744..ef5c1730 100644
--- a/bus/activation.c
+++ b/bus/activation.c
@@ -110,6 +110,8 @@ bus_activation_entry_free (BusActivationEntry *entry)
   
   dbus_free (entry->name);
   dbus_free (entry->exec);
+
+  dbus_free (entry);
 }
 
 static dbus_bool_t
@@ -197,7 +199,8 @@ load_directory (BusActivation *activation,
   DBusString full_path;
   BusDesktopFile *desktop_file;
   DBusError tmp_error;
-
+  dbus_bool_t retval;
+  
   _DBUS_ASSERT_ERROR_IS_CLEAR (error);
   
   _dbus_string_init_const (&dir, directory);
@@ -218,14 +221,16 @@ load_directory (BusActivation *activation,
       return FALSE;
     }
 
-  /* from this point it's safe to "goto failed" */
+  retval = FALSE;
+  
+  /* from this point it's safe to "goto out" */
   
   iter = _dbus_directory_open (&dir, error);
   if (iter == NULL)
     {
       _dbus_verbose ("Failed to open directory %s: %s\n",
                      directory, error ? error->message : "unknown");
-      goto failed;
+      goto out;
     }
   
   /* Now read the files */
@@ -240,7 +245,7 @@ load_directory (BusActivation *activation,
           !_dbus_concat_dir_and_file (&full_path, &filename))
         {
           BUS_SET_OOM (error);
-          goto failed;
+          goto out;
         }
       
       if (!_dbus_string_ends_with_c_str (&filename, ".service"))
@@ -261,7 +266,7 @@ load_directory (BusActivation *activation,
           if (dbus_error_has_name (&tmp_error, DBUS_ERROR_NO_MEMORY))
             {
               dbus_move_error (&tmp_error, error);
-              goto failed;
+              goto out;
             }
           
 	  dbus_error_free (&tmp_error);
@@ -279,7 +284,7 @@ load_directory (BusActivation *activation,
           if (dbus_error_has_name (&tmp_error, DBUS_ERROR_NO_MEMORY))
             {
               dbus_move_error (&tmp_error, error);
-              goto failed;
+              goto out;
             }
 
           dbus_error_free (&tmp_error);
@@ -296,13 +301,16 @@ load_directory (BusActivation *activation,
   if (dbus_error_is_set (&tmp_error))
     {
       dbus_move_error (&tmp_error, error);
-      goto failed;
+      goto out;
     }
   
-  return TRUE;
+  retval = TRUE;
   
- failed:
-  _DBUS_ASSERT_ERROR_IS_SET (error);
+ out:
+  if (!retval)
+    _DBUS_ASSERT_ERROR_IS_SET (error);
+  else
+    _DBUS_ASSERT_ERROR_IS_CLEAR (error);
   
   if (iter != NULL)
     _dbus_directory_close (iter);
@@ -311,7 +319,7 @@ load_directory (BusActivation *activation,
   _dbus_string_free (&filename);
   _dbus_string_free (&full_path);
   
-  return FALSE;
+  return retval;
 }
 
 BusActivation*
diff --git a/configure.in b/configure.in
index fbf60d78..076b74c3 100644
--- a/configure.in
+++ b/configure.in
@@ -403,6 +403,15 @@ fi
 
 AM_CONDITIONAL(DBUS_INIT_SCRIPTS_RED_HAT, test x$with_init_scripts = xredhat)
 
+#### Tell tests where to find certain stuff in builddir
+ABSOLUTE_TOP_BUILDDIR=`cd ${ac_top_builddir}. && pwd`
+
+TEST_SERVICE_BINARY=${ABSOLUTE_TOP_BUILDDIR}/test/test-service
+AC_SUBST(TEST_SERVICE_BINARY)
+
+TEST_SERVICE_DIR=${ABSOLUTE_TOP_BUILDDIR}/test/data/valid-service-files
+AC_SUBST(TEST_SERVICE_DIR)
+
 AC_OUTPUT([
 Doxyfile
 bus/system.conf
@@ -417,6 +426,8 @@ test/Makefile
 doc/Makefile
 dbus-1.0.pc
 dbus-glib-1.0.pc
+test/data/valid-config-files/debug-allow-all.conf
+test/data/valid-service-files/debug-echo.service
 ])
 
 dnl ==========================================================================
diff --git a/dbus/dbus-bus.c b/dbus/dbus-bus.c
index dc1762eb..df883f51 100644
--- a/dbus/dbus-bus.c
+++ b/dbus/dbus-bus.c
@@ -534,7 +534,7 @@ dbus_bus_acquire_service (DBusConnection *connection,
                           DBusError      *error)
 {
   DBusMessage *message, *reply;
-  int service_result;
+  dbus_uint32_t service_result;
   
   message = dbus_message_new (DBUS_SERVICE_DBUS,
                               DBUS_MESSAGE_ACQUIRE_SERVICE);
@@ -564,16 +564,26 @@ dbus_bus_acquire_service (DBusConnection *connection,
     {
       _DBUS_ASSERT_ERROR_IS_SET (error);
       return -1;
-    }
+    }  
 
+  if (dbus_set_error_from_message (error, reply))
+    {
+      _DBUS_ASSERT_ERROR_IS_SET (error);
+      dbus_message_unref (reply);
+      return -1;
+    }
+  
   if (!dbus_message_get_args (reply, error,
                               DBUS_TYPE_UINT32, &service_result,
                               0))
     {
       _DBUS_ASSERT_ERROR_IS_SET (error);
+      dbus_message_unref (reply);
       return -1;
     }
 
+  dbus_message_unref (reply);
+  
   return service_result;
 }
 
diff --git a/dbus/dbus-message.c b/dbus/dbus-message.c
index 39ed3942..46c4c406 100644
--- a/dbus/dbus-message.c
+++ b/dbus/dbus-message.c
@@ -2381,6 +2381,45 @@ dbus_message_sender_is (DBusMessage  *message,
     return FALSE;
 }
 
+/**
+ * Sets a #DBusError based on the contents of the given
+ * message. The error is only set if the message
+ * is an error message, as in dbus_message_get_is_error().
+ * The name of the error is set to the name of the message,
+ * and the error message is set to the first argument
+ * if the argument exists and is a string.
+ *
+ * The return value indicates whether the error was set (the error is
+ * set if and only if the message is an error message).
+ * So you can check for an error reply and convert it to DBusError
+ * in one go.
+ *
+ * @param error the error to set
+ * @param message the message to set it from
+ * @returns #TRUE if dbus_message_get_is_error() returns #TRUE for the message
+ */
+dbus_bool_t
+dbus_set_error_from_message (DBusError   *error,
+                             DBusMessage *message)
+{
+  char *str;
+  
+  if (!dbus_message_get_is_error (message))
+    return FALSE;
+
+  str = NULL;
+  dbus_message_get_args (message, NULL,
+                         DBUS_TYPE_STRING, &str,
+                         DBUS_TYPE_INVALID);
+
+  dbus_set_error (error, dbus_message_get_name (message),
+                  str ? "%s" : NULL, str);
+
+  dbus_free (str);
+  
+  return TRUE;
+}
+
 /** @} */
 
 /**
diff --git a/dbus/dbus-message.h b/dbus/dbus-message.h
index be752c94..47337863 100644
--- a/dbus/dbus-message.h
+++ b/dbus/dbus-message.h
@@ -148,6 +148,11 @@ dbus_bool_t   dbus_message_iter_get_string_array  (DBusMessageIter   *iter,
 dbus_bool_t   dbus_message_iter_get_dict          (DBusMessageIter   *iter,
 						   DBusDict         **dict);
 
+
+
+dbus_bool_t  dbus_set_error_from_message  (DBusError    *error,
+                                           DBusMessage  *message);
+
 DBUS_END_DECLS;
 
 #endif /* DBUS_MESSAGE_H */
diff --git a/doc/TODO b/doc/TODO
index ebbff3ac..f6c539dd 100644
--- a/doc/TODO
+++ b/doc/TODO
@@ -48,3 +48,14 @@
 
  - Abstract the user database, so you can use something other than the system password 
    database.
+
+ - The convenience functions in dbus-bus.h should perhaps have
+   the signatures that they would have if they were autogenerated
+   stubs. e.g. the acquire service function. We should also evaluate 
+   which of these functions to include, in light of the fact that 
+   GLib/Qt native stubs will probably also exist.
+
+ - The message handler interface needs rethinking, perhaps handlers should be able 
+   to return an error that automatically gets turned into a message; most likely 
+   some basic spec'ing out of the GLib/Qt level stubs/skels stuff will be 
+   needed to understand the right approach.
diff --git a/doc/dbus-specification.sgml b/doc/dbus-specification.sgml
index 7e2fcb35..3c41068e 100644
--- a/doc/dbus-specification.sgml
+++ b/doc/dbus-specification.sgml
@@ -184,7 +184,7 @@
             <tbody>
               <row>
                 <entry>0x1</entry>
-                <entry>This message is an error reply.</entry>
+                <entry>This message is an error reply. If the first argument exists and is a string, it is an error message.</entry>
               </row>
             </tbody>
           </tgroup>
@@ -978,12 +978,13 @@
         number of the message being replied to.
       </para>
       <para>
-        If an error occurs, an error reply may be sent in place of the 
-        standard reply. Error replies can be identified by a special 
-        header flag, see <xref linkend="message-protocol-header-encoding">.
-          Error replies have a name which reflects the type of 
-          error that occurred. Error replies would generally 
-          be mapped to exceptions in a programming language.
+        If an error occurs, an error reply may be sent in place of the standard
+        reply. Error replies can be identified by a special header flag, see
+        <xref linkend="message-protocol-header-encoding">.  Error replies have a
+        name which reflects the type of error that occurred. Error replies would
+        generally be mapped to exceptions in a programming language.  If an
+        error reply has a first argument, and that argument has type STRING,
+        then the argument must be an error message.
       </para>
       <para>
         [FIXME discuss mapping of broadcast messages + matching rules 
@@ -1220,7 +1221,7 @@
             # Sample service description file
             [D-BUS Service]
             Name=org.gnome.ConfigurationDatabase
-            Exec=gconfd-2
+            Exec=/usr/libexec/gconfd-2
           </programlisting>
 	</figure>
       </para>
diff --git a/test/Makefile.am b/test/Makefile.am
index 46a66dbc..d0429aaf 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -61,9 +61,10 @@ TESTDIRS=					\
 	data/auth				\
 	data/sha-1				\
 	data/valid-config-files			\
-	data/valid-config-files/basic.d
+	data/valid-config-files/basic.d		\
+	data/valid-service-files
 
-FIND_TESTS=find -name "*.message" -o -name "*.message-raw" -o -name "*.auth-script" -o -name "*.sha1" -o -name "*.txt" -o -name "*.conf"
+FIND_TESTS=find -name "*.message" -o -name "*.message-raw" -o -name "*.auth-script" -o -name "*.sha1" -o -name "*.txt" -o -name "*.conf" -o -name "*.service"
 
 dist-hook:
 	for D in $(TESTDIRS); do						\
diff --git a/test/test-service.c b/test/test-service.c
index a4dff0b3..49048f66 100644
--- a/test/test-service.c
+++ b/test/test-service.c
@@ -1,15 +1,74 @@
 #include <dbus/dbus.h>
 #include <stdio.h>
+#include <stdlib.h>
 #include "watch.h"
 
+static void
+die (const char *message)
+{
+  fprintf (stderr, "%s", message);
+  exit (1);
+}
+
+static DBusHandlerResult
+echo_handler (DBusMessageHandler *handler,
+              DBusConnection     *connection,
+              DBusMessage        *message,
+              void               *user_data)
+{
+  DBusError error;
+  DBusMessage *reply;
+  char *s;
+  
+  dbus_error_init (&error);
+  
+  if (!dbus_message_get_args (message,
+                              &error,
+                              DBUS_TYPE_STRING, &s,
+                              DBUS_TYPE_INVALID))
+    {
+      reply = dbus_message_new_error_reply (message,
+                                            error.name,
+                                            error.message);
+
+      if (reply == NULL)
+        die ("No memory\n");
+
+      if (!dbus_connection_send (connection, reply, NULL))
+        die ("No memory\n");
+
+      dbus_message_unref (reply);
+
+      return DBUS_HANDLER_RESULT_ALLOW_MORE_HANDLERS;
+    }
+
+  reply = dbus_message_new_reply (message);
+  if (reply == NULL)
+    die ("No memory\n");
+
+  if (!dbus_message_append_string (reply, s))
+    die ("No memory");
+
+  if (!dbus_connection_send (connection, reply, NULL))
+    die ("No memory\n");
+  
+  dbus_free (s);
+  
+  dbus_message_unref (reply);
+    
+  return DBUS_HANDLER_RESULT_ALLOW_MORE_HANDLERS;
+}
+
 int
 main (int    argc,
       char **argv)
 {
   DBusConnection *connection;
   DBusError error;
-  DBusMessage *message;
-
+  DBusMessageHandler *handler;
+  const char *to_handle[] = { "org.freedesktop.DBus.TestSuiteEcho" };
+  int result;
+  
   dbus_error_init (&error);
   connection = dbus_bus_get (DBUS_BUS_ACTIVATION, &error);
   if (connection == NULL)
@@ -21,10 +80,31 @@ main (int    argc,
     }
 
   setup_connection (connection);
+
+  handler = dbus_message_handler_new (echo_handler, NULL, NULL);
+  if (handler == NULL)
+    die ("No memory");
+  
+  if (!dbus_connection_register_handler (connection, handler, to_handle, 1))
+    die ("No memory");
+
+  result = dbus_bus_acquire_service (connection, "org.freedesktop.DBus.TestSuiteEchoService",
+                                     0, &error);
+  if (dbus_error_is_set (&error))
+    {
+      fprintf (stderr, "Failed to acquire service: %s\n",
+               error.message);
+      dbus_error_free (&error);
+      return 1;
+    }
   
   do_mainloop ();
 
   dbus_connection_unref (connection);
+
+  dbus_message_handler_unref (handler);
+  
+  dbus_shutdown ();
   
   return 0;
 }