2003-03-28 Havoc Pennington <hp@pobox.com>

	* bus/test.c (bus_test_flush_bus): remove the sleep from here,
	I think it may have just been superstition. Not sure.

	* dbus/dbus-string.c (_dbus_string_base64_decode): catch some OOM
	failures that were not being handled.

	* dbus/dbus-auth.c (process_auth): fix a memleak in OOM handling

	* dbus/dbus-memory.c: add ability to set number of mallocs in a
	row that will fail on out-of-memory.

	* dbus/dbus-internals.c (_dbus_test_oom_handling): convenience
	function for testing out-of-memory handling.

	* bus/config-loader-expat.c (memsuite): don't wrap the dbus
	allocation functions, they do map exactly to the expat ones.

13 files changed, 233 insertions, 119 deletions

diff --git a/ChangeLog b/ChangeLog
index 89813354..9f7ce5ad 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,22 @@
+2003-03-28  Havoc Pennington  <hp@pobox.com>
+
+	* bus/test.c (bus_test_flush_bus): remove the sleep from here, 
+	I think it may have just been superstition. Not sure.
+
+	* dbus/dbus-string.c (_dbus_string_base64_decode): catch some OOM
+	failures that were not being handled.
+
+	* dbus/dbus-auth.c (process_auth): fix a memleak in OOM handling
+
+	* dbus/dbus-memory.c: add ability to set number of mallocs in a
+	row that will fail on out-of-memory.
+
+	* dbus/dbus-internals.c (_dbus_test_oom_handling): convenience
+	function for testing out-of-memory handling.
+
+	* bus/config-loader-expat.c (memsuite): don't wrap the dbus
+	allocation functions, they do map exactly to the expat ones.
+
 2003-03-27  Havoc Pennington  <hp@redhat.com>
 
 	* bus/config-loader-libxml.c (bus_config_load): add another error
diff --git a/bus/config-loader-expat.c b/bus/config-loader-expat.c
index 5e8d28c1..c7981bf4 100644
--- a/bus/config-loader-expat.c
+++ b/bus/config-loader-expat.c
@@ -25,29 +25,11 @@
 #include <dbus/dbus-internals.h>
 #include <expat.h>
 
-static void*
-expat_malloc (size_t size)
-{
-  return dbus_malloc (size);
-}
-
-static void*
-expat_realloc (void *ptr, size_t size)
-{
-  return dbus_realloc (ptr, size);
-}
-
-static void
-expat_free (void *ptr)
-{
-  dbus_free (ptr);
-}
-
 static XML_Memory_Handling_Suite memsuite =
 {
-  expat_malloc,
-  expat_realloc,
-  expat_free
+  dbus_malloc,
+  dbus_realloc,
+  dbus_free
 };
 
 typedef struct
diff --git a/bus/config-parser.c b/bus/config-parser.c
index 8fb3b29e..2429cce5 100644
--- a/bus/config-parser.c
+++ b/bus/config-parser.c
@@ -285,47 +285,18 @@ do_load (const DBusString *full_path,
     }
 }
 
-static dbus_bool_t
-check_oom_loading (const DBusString *full_path,
-                   Validity          validity)
+typedef struct
 {
-  int approx_mallocs;
-
-  /* Run once to see about how many mallocs are involved */
-  
-  _dbus_set_fail_alloc_counter (_DBUS_INT_MAX);
-
-  if (!do_load (full_path, validity, FALSE))
-    return FALSE;
+  const DBusString *full_path;
+  Validity          validity;
+} LoaderOomData;
 
-  approx_mallocs = _DBUS_INT_MAX - _dbus_get_fail_alloc_counter ();
-
-  _dbus_verbose ("=================\nabout %d mallocs total\n=================\n",
-                 approx_mallocs);
-  
-  approx_mallocs += 10; /* fudge factor */
-  
-  /* Now run failing each malloc */
-  
-  while (approx_mallocs >= 0)
-    {
-      
-      _dbus_set_fail_alloc_counter (approx_mallocs);
-
-      _dbus_verbose ("\n===\n(will fail malloc %d)\n===\n",
-                     approx_mallocs);
-
-      if (!do_load (full_path, validity, TRUE))
-        return FALSE;
-      
-      approx_mallocs -= 1;
-    }
-
-  _dbus_set_fail_alloc_counter (_DBUS_INT_MAX);
-
-  _dbus_verbose ("=================\n all iterations passed\n=================\n");
+static dbus_bool_t
+check_loader_oom_func (void *data)
+{
+  LoaderOomData *d = data;
 
-  return TRUE;
+  return do_load (d->full_path, d->validity, TRUE);
 }
 
 static dbus_bool_t
@@ -376,6 +347,7 @@ process_test_subdir (const DBusString *test_base_dir,
   while (_dbus_directory_get_next_file (dir, &filename, &error))
     {
       DBusString full_path;
+      LoaderOomData d;
       
       if (!_dbus_string_init (&full_path, _DBUS_INT_MAX))
         _dbus_assert_not_reached ("couldn't init string");
@@ -407,7 +379,9 @@ process_test_subdir (const DBusString *test_base_dir,
                      (validity == INVALID ? "invalid" :
                       (validity == UNKNOWN ? "unknown" : "???")));
 
-      if (!check_oom_loading (&full_path, validity))
+      d.full_path = &full_path;
+      d.validity = validity;
+      if (!_dbus_test_oom_handling ("config-loader", check_loader_oom_func, &d))
         _dbus_assert_not_reached ("test failed");
       
       _dbus_string_free (&full_path);
diff --git a/bus/dispatch.c b/bus/dispatch.c
index 7db4ac29..2b1dc782 100644
--- a/bus/dispatch.c
+++ b/bus/dispatch.c
@@ -916,49 +916,42 @@ check_hello_connection (BusContext *context)
   return TRUE;
 }
 
-static void
-check1_try_iterations (BusContext *context,
-                       const char *description,
-                       Check1Func  func)
+typedef struct
 {
-  int approx_mallocs;
-
-  /* Run once to see about how many mallocs are involved */
-  
-  _dbus_set_fail_alloc_counter (_DBUS_INT_MAX);
-  
-  if (! (*func) (context))
-    _dbus_assert_not_reached ("test failed");
+  Check1Func func;
+  BusContext *context;
+} Check1Data;
 
-  approx_mallocs = _DBUS_INT_MAX - _dbus_get_fail_alloc_counter ();
+static dbus_bool_t
+check_oom_check1_func (void *data)
+{
+  Check1Data *d = data;
 
-  _dbus_verbose ("=================\n%s: about %d mallocs total\n=================\n",
-                 description, approx_mallocs);
-  
-  approx_mallocs += 10; /* fudge factor */
-  
-  /* Now run failing each malloc */
+  if (! (* d->func) (d->context))
+    return FALSE;
   
-  while (approx_mallocs >= 0)
+  if (!check_no_leftovers (d->context))
     {
-      _dbus_set_fail_alloc_counter (approx_mallocs);
-
-      _dbus_verbose ("\n===\n %s: (will fail malloc %d)\n===\n",
-                     description, approx_mallocs);
+      _dbus_warn ("Messages were left over, should be covered by test suite");
+      return FALSE;
+    }
 
-      if (! (*func) (context))
-        _dbus_assert_not_reached ("test failed");
+  return TRUE;
+}
 
-      if (!check_no_leftovers (context))
-        _dbus_assert_not_reached ("Messages were left over, should be covered by test suite");
-      
-      approx_mallocs -= 1;
-    }
+static void
+check1_try_iterations (BusContext *context,
+                       const char *description,
+                       Check1Func  func)
+{
+  Check1Data d;
 
-  _dbus_set_fail_alloc_counter (_DBUS_INT_MAX);
+  d.func = func;
+  d.context = context;
 
-  _dbus_verbose ("=================\n%s: all iterations passed\n=================\n",
-                 description);
+  if (!_dbus_test_oom_handling (description, check_oom_check1_func,
+                                &d))
+    _dbus_assert_not_reached ("test failed");
 }
 
 dbus_bool_t
diff --git a/bus/test.c b/bus/test.c
index 24cc6efa..ea2c3a19 100644
--- a/bus/test.c
+++ b/bus/test.c
@@ -267,7 +267,6 @@ bus_test_client_listed (DBusConnection *connection)
   return FALSE;
 }
 
-
 void
 bus_test_flush_bus (BusContext *context)
 {
@@ -276,11 +275,14 @@ bus_test_flush_bus (BusContext *context)
    * one end of the debug pipe to come out the other end...
    * a more robust setup would be good. Blocking on the other
    * end of pipes we've pushed data into or something.
+   * A simple hack might be to just make the debug server always
+   * poll for read on the other end of the pipe after writing.
    */
-  
   while (bus_loop_iterate (FALSE))
     ;
+#if 0
   _dbus_sleep_milliseconds (15);
+#endif
   while (bus_loop_iterate (FALSE))
     ;
 }
diff --git a/configure.in b/configure.in
index 7212f3be..81b316df 100644
--- a/configure.in
+++ b/configure.in
@@ -244,8 +244,8 @@ elif test x$with_xml = xlibxml; then
         fi
 else
         ### expat is the default because libxml can't currently survive 
-        ### our brutal OOM-handling unit test setup. Need to find the 
-        ### bugs and submit to DV
+        ### our brutal OOM-handling unit test setup.
+        ### http://bugzilla.gnome.org/show_bug.cgi?id=109368
         if $have_expat ; then
                 with_xml=expat
                 dbus_use_expat=true
diff --git a/dbus/dbus-auth.c b/dbus/dbus-auth.c
index 8f8aec44..1aa83e7c 100644
--- a/dbus/dbus-auth.c
+++ b/dbus/dbus-auth.c
@@ -1241,14 +1241,14 @@ process_auth (DBusAuth         *auth,
           _dbus_string_free (&mech);
           return FALSE;
         }
-
+      
       if (!_dbus_string_init (&decoded_response, _DBUS_INT_MAX))
         {
           _dbus_string_free (&mech);
           _dbus_string_free (&base64_response);
           return FALSE;
         }
-      
+
       if (!_dbus_string_copy_len (args, 0, i, &mech, 0))
         goto failed;
 
@@ -1258,7 +1258,7 @@ process_auth (DBusAuth         *auth,
       if (!_dbus_string_base64_decode (&base64_response, 0,
                                        &decoded_response, 0))
         goto failed;
-
+      
       auth->mech = find_mech (&mech);
       if (auth->mech != NULL)
         {
@@ -1274,7 +1274,7 @@ process_auth (DBusAuth         *auth,
         {
           /* Unsupported mechanism */
           if (!send_rejected (auth))
-            return FALSE;
+            goto failed;
         }
 
       _dbus_string_free (&mech);      
diff --git a/dbus/dbus-bus.h b/dbus/dbus-bus.h
index c6800ede..9f25eeb6 100644
--- a/dbus/dbus-bus.h
+++ b/dbus/dbus-bus.h
@@ -29,6 +29,8 @@
 
 #include <dbus/dbus-connection.h>
 
+DBUS_BEGIN_DECLS;
+
 dbus_bool_t dbus_bus_register         (DBusConnection *connection,
                                        DBusError      *error);
 dbus_bool_t dbus_bus_set_base_service (DBusConnection *connection,
@@ -42,6 +44,6 @@ dbus_bool_t dbus_bus_service_exists   (DBusConnection *connection,
                                        const char     *service_name,
                                        DBusError      *error);
 
-
+DBUS_END_DECLS;
 
 #endif /* DBUS_BUS_H */
diff --git a/dbus/dbus-internals.c b/dbus/dbus-internals.c
index 9588e72b..1c018b7f 100644
--- a/dbus/dbus-internals.c
+++ b/dbus/dbus-internals.c
@@ -292,4 +292,85 @@ _dbus_type_to_string (int type)
     }
 }
 
+static dbus_bool_t
+run_failing_each_malloc (int                    n_mallocs,
+                         const char            *description,
+                         DBusTestMemoryFunction func,
+                         void                  *data)
+{
+  n_mallocs += 10; /* fudge factor to ensure reallocs etc. are covered */
+  
+  while (n_mallocs >= 0)
+    {      
+      _dbus_set_fail_alloc_counter (n_mallocs);
+
+      _dbus_verbose ("\n===\n%s: (will fail malloc %d with %d failures)\n===\n",
+                     description, n_mallocs,
+                     _dbus_get_fail_alloc_failures ());
+
+      if (!(* func) (data))
+        return FALSE;
+      
+      n_mallocs -= 1;
+    }
+
+  _dbus_set_fail_alloc_counter (_DBUS_INT_MAX);
+
+  return TRUE;
+}                        
+
+/**
+ * Tests how well the given function responds to out-of-memory
+ * situations. Calls the function repeatedly, failing a different
+ * call to malloc() each time. If the function ever returns #FALSE,
+ * the test fails. The function should return #TRUE whenever something
+ * valid (such as returning an error, or succeeding) occurs, and #FALSE
+ * if it gets confused in some way.
+ *
+ * @param description description of the test used in verbose output
+ * @param func function to call
+ * @param data data to pass to function
+ * @returns #TRUE if the function never returns FALSE
+ */
+dbus_bool_t
+_dbus_test_oom_handling (const char             *description,
+                         DBusTestMemoryFunction  func,
+                         void                   *data)
+{
+  int approx_mallocs;
+
+  /* Run once to see about how many mallocs are involved */
+  
+  _dbus_set_fail_alloc_counter (_DBUS_INT_MAX);
+
+  if (!(* func) (data))
+    return FALSE;
+
+  approx_mallocs = _DBUS_INT_MAX - _dbus_get_fail_alloc_counter ();
+
+  _dbus_verbose ("=================\n%s: about %d mallocs total\n=================\n",
+                 description, approx_mallocs);
+
+  _dbus_set_fail_alloc_failures (1);
+  if (!run_failing_each_malloc (approx_mallocs, description, func, data))
+    return FALSE;
+
+  _dbus_set_fail_alloc_failures (2);
+  if (!run_failing_each_malloc (approx_mallocs, description, func, data))
+    return FALSE;
+  
+  _dbus_set_fail_alloc_failures (3);
+  if (!run_failing_each_malloc (approx_mallocs, description, func, data))
+    return FALSE;
+
+  _dbus_set_fail_alloc_failures (4);
+  if (!run_failing_each_malloc (approx_mallocs, description, func, data))
+    return FALSE;
+  
+  _dbus_verbose ("=================\n%s: all iterations passed\n=================\n",
+                 description);
+
+  return TRUE;
+}
+
 /** @} */
diff --git a/dbus/dbus-internals.h b/dbus/dbus-internals.h
index d95e58d6..9bfd0b3b 100644
--- a/dbus/dbus-internals.h
+++ b/dbus/dbus-internals.h
@@ -170,9 +170,16 @@ extern const char _dbus_no_memory_message[];
 /* Memory debugging */
 void        _dbus_set_fail_alloc_counter        (int  until_next_fail);
 int         _dbus_get_fail_alloc_counter        (void);
+void        _dbus_set_fail_alloc_failures       (int  failures_per_failure);
+int         _dbus_get_fail_alloc_failures       (void);
 dbus_bool_t _dbus_decrement_fail_alloc_counter  (void);
 dbus_bool_t _dbus_disable_mem_pools             (void);
 int         _dbus_get_malloc_blocks_outstanding (void);
+
+typedef dbus_bool_t (* DBusTestMemoryFunction)  (void *data);
+dbus_bool_t _dbus_test_oom_handling (const char             *description,
+                                     DBusTestMemoryFunction  func,
+                                     void                   *data);
 #else
 #define _dbus_set_fail_alloc_counter(n)
 #define _dbus_get_fail_alloc_counter _DBUS_INT_MAX
diff --git a/dbus/dbus-memory.c b/dbus/dbus-memory.c
index 983c6e31..11d52045 100644
--- a/dbus/dbus-memory.c
+++ b/dbus/dbus-memory.c
@@ -76,9 +76,11 @@
 
 #ifdef DBUS_BUILD_TESTS
 static dbus_bool_t debug_initialized = FALSE;
-static int fail_counts = -1;
+static int fail_nth = -1;
 static size_t fail_size = 0;
 static int fail_alloc_counter = _DBUS_INT_MAX;
+static int n_failures_per_failure = 1;
+static int n_failures_this_failure = 0;
 static dbus_bool_t guards = FALSE;
 static dbus_bool_t disable_mem_pools = FALSE;
 static dbus_bool_t backtrace_on_fail_alloc = FALSE;
@@ -106,9 +108,9 @@ _dbus_initialize_malloc_debug (void)
       
       if (_dbus_getenv ("DBUS_MALLOC_FAIL_NTH") != NULL)
 	{
-	  fail_counts = atoi (_dbus_getenv ("DBUS_MALLOC_FAIL_NTH"));
-          fail_alloc_counter = fail_counts;
-          _dbus_verbose ("Will fail malloc every %d times\n", fail_counts);
+	  fail_nth = atoi (_dbus_getenv ("DBUS_MALLOC_FAIL_NTH"));
+          fail_alloc_counter = fail_nth;
+          _dbus_verbose ("Will fail malloc every %d times\n", fail_nth);
 	}
       
       if (_dbus_getenv ("DBUS_MALLOC_FAIL_GREATER_THAN") != NULL)
@@ -185,6 +187,30 @@ _dbus_get_fail_alloc_counter (void)
 }
 
 /**
+ * Sets how many mallocs to fail when the fail alloc counter reaches
+ * 0.
+ *
+ * @param number to fail
+ */
+void
+_dbus_set_fail_alloc_failures (int failures_per_failure)
+{
+  n_failures_per_failure = failures_per_failure;
+}
+
+/**
+ * Gets the number of failures we'll have when the fail malloc
+ * counter reaches 0.
+ *
+ * @returns number of failures planned
+ */
+int
+_dbus_get_fail_alloc_failures (void)
+{
+  return n_failures_per_failure;
+}
+
+/**
  * Called when about to alloc some memory; if
  * it returns #TRUE, then the allocation should
  * fail. If it returns #FALSE, then the allocation
@@ -199,14 +225,23 @@ _dbus_decrement_fail_alloc_counter (void)
   
   if (fail_alloc_counter <= 0)
     {
-      if (fail_counts >= 0)
-        fail_alloc_counter = fail_counts;
-      else
-        fail_alloc_counter = _DBUS_INT_MAX;
-
-      _dbus_verbose ("reset fail alloc counter to %d\n", fail_alloc_counter);
       if (backtrace_on_fail_alloc)
         _dbus_print_backtrace ();
+
+      _dbus_verbose ("failure %d\n", n_failures_this_failure);
+      
+      n_failures_this_failure += 1;
+      if (n_failures_this_failure >= n_failures_per_failure)
+        {
+          if (fail_nth >= 0)
+            fail_alloc_counter = fail_nth;
+          else
+            fail_alloc_counter = _DBUS_INT_MAX;
+
+          n_failures_this_failure = 0;
+
+          _dbus_verbose ("reset fail alloc counter to %d\n", fail_alloc_counter);
+        }
       
       return TRUE;
     }
diff --git a/dbus/dbus-string.c b/dbus/dbus-string.c
index 1bc3e205..7549dcad 100644
--- a/dbus/dbus-string.c
+++ b/dbus/dbus-string.c
@@ -2108,15 +2108,22 @@ _dbus_string_base64_decode (const DBusString *source,
 	       */
               
               if (pad_count < 1)
-                _dbus_string_append_byte (&result,
-                                          triplet >> 16);
+                {
+                  if (!_dbus_string_append_byte (&result,
+                                                 triplet >> 16))
+                    goto failed;
+                }
               
               if (pad_count < 2)
-                _dbus_string_append_byte (&result,
-                                          (triplet >> 8) & 0xff);              
+                {
+                  if (!_dbus_string_append_byte (&result,
+                                                 (triplet >> 8) & 0xff))
+                    goto failed;
+                }
               
-              _dbus_string_append_byte (&result,
-                                        triplet & 0xff);
+              if (!_dbus_string_append_byte (&result,
+                                             triplet & 0xff))
+                goto failed;
               
               sextet_count = 0;
               pad_count = 0;
@@ -2136,6 +2143,11 @@ _dbus_string_base64_decode (const DBusString *source,
   _dbus_string_free (&result);
 
   return TRUE;
+
+ failed:
+  _dbus_string_free (&result);
+
+  return FALSE;
 }
 
 /**
diff --git a/doc/TODO b/doc/TODO
index 1296f92a..001e0a5e 100644
--- a/doc/TODO
+++ b/doc/TODO
@@ -39,3 +39,10 @@
 
  - Automatic service activation, should probably be done through a message flag.
 
+ - Disconnecting the remote end on invalid UTF-8 is probably not a good 
+   idea. The definitiion of "valid" is slightly fuzzy. I think it might 
+   be better to just silently "fix" the UTF-8, or perhaps return an error.
+
+ - We might consider returning a "no such operation" error in dbus-connection.c 
+   for unhandled messages.
+