2005-01-15 Havoc Pennington <hp@redhat.com>

	* test/glib/test-profile.c (with_bus_server_filter): fix crash

	* dbus/dbus-marshal-basic.c (_dbus_unpack_uint32): inline as macro
	when DBUS_DISABLE_ASSERT
	(_dbus_marshal_set_basic): be sure we align for the string length

	* dbus/dbus-marshal-recursive.c (skip_one_complete_type): make
	this look faster

	* dbus/dbus-string.c (_dbus_string_get_const_data_len): add an
	inline macro version
	(_dbus_string_set_byte): provide inline macro version

6 files changed, 101 insertions, 69 deletions

diff --git a/dbus/dbus-marshal-basic.c b/dbus/dbus-marshal-basic.c
index 7504019b..a2e32752 100644
--- a/dbus/dbus-marshal-basic.c
+++ b/dbus/dbus-marshal-basic.c
@@ -103,18 +103,6 @@ _dbus_pack_int32 (dbus_int32_t   value,
   pack_4_octets ((dbus_uint32_t) value, byte_order, data);
 }
 
-static dbus_uint32_t
-unpack_4_octets (int                  byte_order,
-                 const unsigned char *data)
-{
-  _dbus_assert (_DBUS_ALIGN_ADDRESS (data, 4) == data);
-
-  if (byte_order == DBUS_LITTLE_ENDIAN)
-    return DBUS_UINT32_FROM_LE (*(dbus_uint32_t*)data);
-  else
-    return DBUS_UINT32_FROM_BE (*(dbus_uint32_t*)data);
-}
-
 #ifndef DBUS_HAVE_INT64
 /* from ORBit */
 static void
@@ -174,6 +162,7 @@ unpack_8_octets (int                  byte_order,
 }
 #endif
 
+#ifndef _dbus_unpack_uint32
 /**
  * Unpacks a 32 bit unsigned integer from a data pointer
  *
@@ -185,8 +174,14 @@ dbus_uint32_t
 _dbus_unpack_uint32 (int                  byte_order,
                      const unsigned char *data)
 {
-  return unpack_4_octets (byte_order, data);
+  _dbus_assert (_DBUS_ALIGN_ADDRESS (data, 4) == data);
+
+  if (byte_order == DBUS_LITTLE_ENDIAN)
+    return DBUS_UINT32_FROM_LE (*(dbus_uint32_t*)data);
+  else
+    return DBUS_UINT32_FROM_BE (*(dbus_uint32_t*)data);
 }
+#endif /* _dbus_unpack_uint32 */
 
 /**
  * Unpacks a 32 bit signed integer from a data pointer
@@ -199,7 +194,7 @@ dbus_int32_t
 _dbus_unpack_int32 (int                  byte_order,
                     const unsigned char *data)
 {
-  return (dbus_int32_t) unpack_4_octets (byte_order, data);
+  return (dbus_int32_t) _dbus_unpack_uint32 (byte_order, data);
 }
 
 static void
@@ -285,7 +280,9 @@ set_string (DBusString          *str,
 
   _dbus_string_init_const (&dstr, value);
 
-  old_len = _dbus_marshal_read_uint32 (str, pos, byte_order, NULL);
+  _dbus_assert (_DBUS_ALIGN_VALUE (pos, 4) == pos);
+  old_len = _dbus_unpack_uint32 (byte_order,
+                                 _dbus_string_get_const_data_len (str, pos, 4));
 
   new_len = _dbus_string_get_length (&dstr);
 
@@ -406,6 +403,7 @@ _dbus_marshal_set_basic (DBusString       *str,
       break;
     case DBUS_TYPE_STRING:
     case DBUS_TYPE_OBJECT_PATH:
+      pos = _DBUS_ALIGN_VALUE (pos, 4);
       _dbus_assert (vp->str != NULL);
       return set_string (str, pos, vp->str, byte_order,
                          old_end_pos, new_end_pos);
@@ -422,23 +420,6 @@ _dbus_marshal_set_basic (DBusString       *str,
     }
 }
 
-static dbus_uint32_t
-read_4_octets (const DBusString *str,
-               int               pos,
-               int               byte_order,
-               int              *new_pos)
-{
-  pos = _DBUS_ALIGN_VALUE (pos, 4);
-
-  if (new_pos)
-    *new_pos = pos + 4;
-
-  _dbus_assert (pos + 4 <= _dbus_string_get_length (str));
-  
-  return unpack_4_octets (byte_order,
-                          _dbus_string_get_const_data (str) + pos);
-}
-
 /**
  * Convenience function to demarshal a 32 bit unsigned integer.
  *
@@ -454,7 +435,15 @@ _dbus_marshal_read_uint32  (const DBusString *str,
                             int               byte_order,
                             int              *new_pos)
 {
-  return read_4_octets (str, pos, byte_order, new_pos);
+  pos = _DBUS_ALIGN_VALUE (pos, 4);
+
+  if (new_pos)
+    *new_pos = pos + 4;
+
+  _dbus_assert (pos + 4 <= _dbus_string_get_length (str));
+  
+  return _dbus_unpack_uint32 (byte_order,
+                              _dbus_string_get_const_data (str) + pos);
 }
 
 /**
diff --git a/dbus/dbus-marshal-basic.h b/dbus/dbus-marshal-basic.h
index ddfce18e..cf86e713 100644
--- a/dbus/dbus-marshal-basic.h
+++ b/dbus/dbus-marshal-basic.h
@@ -136,6 +136,13 @@ typedef union
   char *str;
 } DBusBasicValue;
 
+#ifdef DBUS_DISABLE_ASSERT
+#define _dbus_unpack_uint32(byte_order, data)           \
+   (((byte_order) == DBUS_LITTLE_ENDIAN) ?              \
+     DBUS_UINT32_FROM_LE (*(dbus_uint32_t*)(data)) :    \
+     DBUS_UINT32_FROM_BE (*(dbus_uint32_t*)(data)))
+#endif
+
 void          _dbus_pack_int32    (dbus_int32_t         value,
                                    int                  byte_order,
                                    unsigned char       *data);
@@ -144,9 +151,10 @@ dbus_int32_t  _dbus_unpack_int32  (int                  byte_order,
 void          _dbus_pack_uint32   (dbus_uint32_t        value,
                                    int                  byte_order,
                                    unsigned char       *data);
+#ifndef _dbus_unpack_uint32
 dbus_uint32_t _dbus_unpack_uint32 (int                  byte_order,
                                    const unsigned char *data);
-
+#endif
 
 dbus_bool_t   _dbus_marshal_set_basic         (DBusString       *str,
                                                int               pos,
diff --git a/dbus/dbus-marshal-header.c b/dbus/dbus-marshal-header.c
index 34940429..58ba86fe 100644
--- a/dbus/dbus-marshal-header.c
+++ b/dbus/dbus-marshal-header.c
@@ -328,14 +328,16 @@ set_basic_field (DBusTypeReader       *reader,
 {
   DBusTypeReader sub;
   DBusTypeReader variant;
-  unsigned char v_BYTE;
 
   _dbus_type_reader_recurse (reader, &sub);
 
   _dbus_assert (_dbus_type_reader_get_current_type (&sub) == DBUS_TYPE_BYTE);
 #ifndef DBUS_DISABLE_ASSERT
-  _dbus_type_reader_read_basic (&sub, &v_BYTE);
-  _dbus_assert (((int) v_BYTE) == field);
+ {
+   unsigned char v_BYTE;
+   _dbus_type_reader_read_basic (&sub, &v_BYTE);
+   _dbus_assert (((int) v_BYTE) == field);
+ }
 #endif
 
   if (!_dbus_type_reader_next (&sub))
diff --git a/dbus/dbus-marshal-recursive.c b/dbus/dbus-marshal-recursive.c
index 3b2ce91d..a8bad46a 100644
--- a/dbus/dbus-marshal-recursive.c
+++ b/dbus/dbus-marshal-recursive.c
@@ -119,7 +119,7 @@ static int
 first_type_in_signature (const DBusString *str,
                          int               pos)
 {
-  int t;
+  unsigned char t;
 
   t = _dbus_string_get_byte (str, pos);
 
@@ -213,13 +213,10 @@ array_reader_get_array_len (const DBusTypeReader *reader)
 
   len_pos = ARRAY_READER_LEN_POS (reader);
 
-  _dbus_marshal_read_basic (reader->value_str,
-                            len_pos,
-                            DBUS_TYPE_UINT32,
-                            &array_len,
-                            reader->byte_order,
-                            NULL);
-
+  _dbus_assert (_DBUS_ALIGN_VALUE (len_pos, 4) == (unsigned) len_pos);
+  array_len = _dbus_unpack_uint32 (reader->byte_order,
+                                   _dbus_string_get_const_data_len (reader->value_str, len_pos, 4));
+  
 #if RECURSIVE_MARSHAL_READ_TRACE
   _dbus_verbose ("   reader %p len_pos %d array len %u len_offset %d\n",
                  reader, len_pos, array_len, reader->array_len_offset);
@@ -311,37 +308,55 @@ array_reader_check_finished (const DBusTypeReader *reader)
   return reader->value_pos == end_pos;
 }
 
+/* this is written a little oddly to try and overoptimize */
 static void
 skip_one_complete_type (const DBusString *type_str,
                         int              *type_pos)
 {
-  while (_dbus_string_get_byte (type_str, *type_pos) == DBUS_TYPE_ARRAY)
-    *type_pos += 1;
+  const unsigned char *p;
+  const unsigned char *start;
+  
+  start = _dbus_string_get_const_data (type_str);
+  p = start + *type_pos;
 
-  if (_dbus_string_get_byte (type_str, *type_pos) == DBUS_STRUCT_BEGIN_CHAR)
+  while (*p == DBUS_TYPE_ARRAY)
+    ++p;
+  
+  if (*p == DBUS_STRUCT_BEGIN_CHAR)
     {
       int depth;
+      
       depth = 1;
-      *type_pos += 1;
-      while (depth > 0)
+      
+      while (TRUE)
         {
-          switch (_dbus_string_get_byte (type_str, *type_pos))
+          _dbus_assert (*p != DBUS_TYPE_INVALID);
+          
+          ++p;
+
+          _dbus_assert (*p != DBUS_TYPE_INVALID);
+          
+          if (*p == DBUS_STRUCT_BEGIN_CHAR)
+            depth += 1;
+          else if (*p == DBUS_STRUCT_END_CHAR)
             {
-            case DBUS_STRUCT_BEGIN_CHAR:
-              depth += 1;
-              break;
-            case DBUS_STRUCT_END_CHAR:
               depth -= 1;
-              break;
-            case DBUS_TYPE_INVALID:
-              _dbus_assert_not_reached ("unbalanced parens in signature");
-              break;
+              if (depth == 0)
+                {
+                  ++p;
+                  break;
+                }
             }
-          *type_pos += 1;
         }
     }
   else
-    *type_pos += 1;
+    {
+      ++p;
+    }
+
+  _dbus_assert (*p != DBUS_STRUCT_END_CHAR);
+
+  *type_pos = (int) (p - start);
 }
 
 static int
@@ -1831,10 +1846,13 @@ writer_recurse_array (DBusTypeWriter   *writer,
         {
           dbus_uint32_t len;
 
-          len = _dbus_marshal_read_uint32 (sub->value_str,
-                                           sub->u.array.len_pos,
-                                           sub->byte_order, NULL);
-
+          _dbus_assert (_DBUS_ALIGN_VALUE (sub->u.array.len_pos, 4) ==
+                        (unsigned) sub->u.array.len_pos);
+          len = _dbus_unpack_uint32 (sub->byte_order,
+                                     _dbus_string_get_const_data_len (sub->value_str,
+                                                                      sub->u.array.len_pos,
+                                                                      4));
+          
           sub->value_pos += len;
         }
     }
@@ -2432,9 +2450,12 @@ writer_write_reader_helper (DBusTypeWriter       *writer,
                 start_after_new_len +
                 bytes_written_after_start_after;
 
-              old_len = _dbus_marshal_read_uint32 (reader->value_str,
-                                                   fixup.len_pos_in_reader,
-                                                   reader->byte_order, NULL);
+              _dbus_assert (_DBUS_ALIGN_VALUE (fixup.len_pos_in_reader, 4) ==
+                            (unsigned) fixup.len_pos_in_reader);
+              
+              old_len = _dbus_unpack_uint32 (reader->byte_order,
+                                             _dbus_string_get_const_data_len (reader->value_str,
+                                                                              fixup.len_pos_in_reader, 4));
 
               if (old_len != fixup.new_len && !append_fixup (fixups, &fixup))
                 goto oom;
diff --git a/dbus/dbus-string.c b/dbus/dbus-string.c
index 777461ed..dccf5176 100644
--- a/dbus/dbus-string.c
+++ b/dbus/dbus-string.c
@@ -1,7 +1,7 @@
 /* -*- mode: C; c-file-style: "gnu" -*- */
 /* dbus-string.c String utility class (internal to D-BUS implementation)
  * 
- * Copyright (C) 2002, 2003, 2004 Red Hat, Inc.
+ * Copyright (C) 2002, 2003, 2004, 2005 Red Hat, Inc.
  *
  * Licensed under the Academic Free License version 2.1
  * 
@@ -517,6 +517,8 @@ _dbus_string_get_data_len (DBusString *str,
   return real->str + start;
 }
 
+/* only do the function if we don't have the macro */
+#ifndef _dbus_string_get_const_data_len
 /**
  * const version of _dbus_string_get_data_len().
  *
@@ -538,7 +540,10 @@ _dbus_string_get_const_data_len (const DBusString  *str,
   
   return real->str + start;
 }
+#endif /* _dbus_string_get_const_data_len */
 
+/* only do the function if we don't have the macro */
+#ifndef _dbus_string_set_byte
 /**
  * Sets the value of the byte at the given position.
  *
@@ -557,6 +562,7 @@ _dbus_string_set_byte (DBusString    *str,
   
   real->str[i] = byte;
 }
+#endif /* _dbus_string_set_byte */
 
 /* only have the function if we didn't create a macro */
 #ifndef _dbus_string_get_byte
diff --git a/dbus/dbus-string.h b/dbus/dbus-string.h
index f0ae1e65..df5f4232 100644
--- a/dbus/dbus-string.h
+++ b/dbus/dbus-string.h
@@ -55,8 +55,10 @@ struct DBusString
  * Note that these break type safety (due to the casts)
  */
 #define _dbus_string_get_length(s) (((DBusString*)(s))->dummy2)
+#define _dbus_string_set_byte(s, i, b) ((((unsigned char*)(((DBusString*)(s))->dummy1))[(i)]) = (unsigned char) (b))
 #define _dbus_string_get_byte(s, i) (((const unsigned char*)(((DBusString*)(s))->dummy1))[(i)])
 #define _dbus_string_get_const_data(s) ((const char*)(((DBusString*)(s))->dummy1))
+#define _dbus_string_get_const_data_len(s,start,len) (((const char*)(((DBusString*)(s))->dummy1)) + (start))
 #endif
 
 dbus_bool_t   _dbus_string_init                  (DBusString        *str);
@@ -76,12 +78,16 @@ const char*   _dbus_string_get_const_data        (const DBusString  *str);
 char*         _dbus_string_get_data_len          (DBusString        *str,
                                                   int                start,
                                                   int                len);
+#ifndef _dbus_string_get_const_data_len
 const char*   _dbus_string_get_const_data_len    (const DBusString  *str,
                                                   int                start,
                                                   int                len);
+#endif
+#ifndef _dbus_string_set_byte
 void          _dbus_string_set_byte              (DBusString        *str,
                                                   int                i,
                                                   unsigned char      byte);
+#endif
 #ifndef _dbus_string_get_byte
 unsigned char _dbus_string_get_byte              (const DBusString  *str,
                                                   int                start);