Improved dbus_string_replace_len()

Reviewed-by: Havoc Pennington <hp@pobox.com>
Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=21261

diff --git a/dbus/dbus-string-util.c b/dbus/dbus-string-util.c
index 4d42bb0..b31703c 100644 (file)
--- a/dbus/dbus-string-util.c
+++ b/dbus/dbus-string-util.c
@@ -266,7 +266,7 @@ _dbus_string_test (void)
 {
   DBusString str;
   DBusString other;
-  int i, end;
+  int i, a, end;
   long v;
   double d;
   int lens[] = { 0, 1, 2, 3, 4, 5, 10, 16, 17, 18, 25, 31, 32, 33, 34, 35, 63, 64, 65, 66, 67, 68, 69, 70, 71, 127, 128, 129, 130, 131, 132, 133, 134, 135, 136 };
@@ -513,10 +513,94 @@ _dbus_string_test (void)
   _dbus_assert (_dbus_string_get_length (&other) == i * 2 - 1);
   _dbus_assert (_dbus_string_equal_c_str (&other,
                                           "HelloHello WorldWorle"));
-  
+
   _dbus_string_free (&str);
   _dbus_string_free (&other);
+
+  /* Different tests are provided because different behaviours are
+   * implemented in _dbus_string_replace_len() in function of replacing and
+   * replaced lengths
+   */
+
+  if (!_dbus_string_init (&str))
+    _dbus_assert_not_reached ("failed to init string");
   
+  if (!_dbus_string_append (&str, "Hello World"))
+    _dbus_assert_not_reached ("could not append to string");
+
+  i = _dbus_string_get_length (&str);
+  
+  if (!_dbus_string_init (&other))
+    _dbus_assert_not_reached ("could not init string");
+
+  if (!_dbus_string_append (&other, "Foo String"))
+    _dbus_assert_not_reached ("could not append to string");
+
+  a = _dbus_string_get_length (&other);
+
+  if (!_dbus_string_replace_len (&str, 0, 6,
+                                 &other, 4, 0))
+    _dbus_assert_not_reached ("could not replace 0 length");
+
+  _dbus_assert (_dbus_string_get_length (&str) == i);
+  _dbus_assert (_dbus_string_get_length (&other) == a + 6);
+  _dbus_assert (_dbus_string_equal_c_str (&other,
+                                          "Foo Hello String"));
+
+  if (!_dbus_string_replace_len (&str, 5, 6,
+                                 &other,
+                                 _dbus_string_get_length (&other),
+                                 0))
+    _dbus_assert_not_reached ("could not replace at the end");
+
+  _dbus_assert (_dbus_string_get_length (&str) == i);
+  _dbus_assert (_dbus_string_get_length (&other) == a + 6 + 6);
+  _dbus_assert (_dbus_string_equal_c_str (&other,
+                                          "Foo Hello String World"));
+
+  if (!_dbus_string_replace_len (&str, 0, 5,
+                                 &other,
+                                 _dbus_string_get_length (&other) - 5,
+                                 5))
+    _dbus_assert_not_reached ("could not replace same length");
+
+  _dbus_assert (_dbus_string_get_length (&str) == i);
+  _dbus_assert (_dbus_string_get_length (&other) == a + 6 + 6);
+  _dbus_assert (_dbus_string_equal_c_str (&other,
+                                          "Foo Hello String Hello"));
+
+  if (!_dbus_string_replace_len (&str, 6, 5,
+                                 &other, 4, 12))
+    _dbus_assert_not_reached ("could not replace with shorter string");
+
+  _dbus_assert (_dbus_string_get_length (&str) == i);
+  _dbus_assert (_dbus_string_get_length (&other) == a + 5);
+  _dbus_assert (_dbus_string_equal_c_str (&other,
+                                          "Foo World Hello"));
+
+  if (!_dbus_string_replace_len (&str, 0, 1,
+                                 &other, 0, 3))
+    _dbus_assert_not_reached ("could not replace at the beginning");
+
+  _dbus_assert (_dbus_string_get_length (&str) == i);
+  _dbus_assert (_dbus_string_get_length (&other) == a + 3);
+  _dbus_assert (_dbus_string_equal_c_str (&other,
+                                          "H World Hello"));
+
+  if (!_dbus_string_replace_len (&str, 6, 5,
+                                 &other,
+                                 _dbus_string_get_length (&other) - 5,
+                                 5))
+    _dbus_assert_not_reached ("could not replace same length");
+
+  _dbus_assert (_dbus_string_get_length (&str) == i);
+  _dbus_assert (_dbus_string_get_length (&other) == a + 3);
+  _dbus_assert (_dbus_string_equal_c_str (&other,
+                                          "H World World"));
+
+  _dbus_string_free (&str);
+  _dbus_string_free (&other);
+
   /* Check append/get unichar */
   
   if (!_dbus_string_init (&str))

diff --git a/dbus/dbus-string.c b/dbus/dbus-string.c
index e2eb93b..2471f46 100644 (file)
--- a/dbus/dbus-string.c
+++ b/dbus/dbus-string.c
@@ -1635,15 +1635,6 @@ _dbus_string_copy_len (const DBusString *source,
 /**
  * Replaces a segment of dest string with a segment of source string.
  *
- * @todo optimize the case where the two lengths are the same, and
- * avoid memmoving the data in the trailing part of the string twice.
- *
- * @todo avoid inserting the source into dest, then deleting
- * the replaced chunk of dest (which creates a potentially large
- * intermediate string). Instead, extend the replaced chunk
- * of dest with padding to the same size as the source chunk,
- * then copy in the source bytes.
- * 
  * @param source the source string
  * @param start where to start copying the source string
  * @param len length of segment to copy
@@ -1669,11 +1660,37 @@ _dbus_string_replace_len (const DBusString *source,
   _dbus_assert (replace_at <= real_dest->len);
   _dbus_assert (replace_len <= real_dest->len - replace_at);
 
-  if (!copy (real_source, start, len,
-             real_dest, replace_at))
-    return FALSE;
+  if (len == replace_len)
+    {
+      memmove (real_dest->str + replace_at,
+               real_source->str + start, len);
+    }
+  else if (len < replace_len)
+    {
+      memmove (real_dest->str + replace_at,
+               real_source->str + start, len);
+      delete (real_dest, replace_at + len,
+              replace_len - len);
+    }
+  else
+    {
+      int diff;
 
-  delete (real_dest, replace_at + len, replace_len);
+      _dbus_assert (len > replace_len);
+
+      diff = len - replace_len;
+
+      /* First of all we check if destination string can be enlarged as
+       * required, then we overwrite previous bytes
+       */
+
+      if (!copy (real_source, start + replace_len, diff,
+                 real_dest, replace_at + replace_len))
+        return FALSE;
+
+      memmove (real_dest->str + replace_at,
+               real_source->str + start, replace_len);
+    }
 
   return TRUE;
 }