2003-04-16 Havoc Pennington <hp@pobox.com>

	* doc/dbus-specification.sgml: make spec say serials are unsigned

	* dbus/dbus-message.h: change message serials to unsigned

	* dbus/dbus-connection.c: adapt to message serials being unsigned

8 files changed, 175 insertions, 89 deletions

diff --git a/ChangeLog b/ChangeLog
index 97c58800..069734da 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,11 @@
+2003-04-16  Havoc Pennington  <hp@pobox.com>
+
+	* doc/dbus-specification.sgml: make spec say serials are unsigned
+
+	* dbus/dbus-message.h: change message serials to unsigned
+
+	* dbus/dbus-connection.c: adapt to message serials being unsigned
+	
 2003-04-15  Havoc Pennington  <hp@pobox.com>
 
 	* bus/bus.c: create and keep around a shared DBusUserDatabase 
diff --git a/bus/connection.c b/bus/connection.c
index c311f7ed..31fe27a0 100644
--- a/bus/connection.c
+++ b/bus/connection.c
@@ -951,7 +951,7 @@ bus_transaction_send (BusTransaction *transaction,
 
   _dbus_verbose ("  trying to add %s %s to transaction%s\n",
                  dbus_message_get_is_error (message) ? "error" :
-                 dbus_message_get_reply_serial (message) != -1 ? "reply" :
+                 dbus_message_get_reply_serial (message) != 0 ? "reply" :
                  "message",
                  dbus_message_get_name (message),
                  dbus_connection_get_is_connected (connection) ?
diff --git a/bus/driver.c b/bus/driver.c
index c7d66d53..0d8b9442 100644
--- a/bus/driver.c
+++ b/bus/driver.c
@@ -620,7 +620,7 @@ bus_driver_handle_message (DBusConnection *connection,
       return FALSE;
     }
 
-  if (dbus_message_get_reply_serial (message) != -1)
+  if (dbus_message_get_reply_serial (message) != 0)
     {
       _dbus_verbose ("Client sent a reply to the bus driver, ignoring it\n");
       return TRUE;
diff --git a/dbus/dbus-connection.c b/dbus/dbus-connection.c
index da7a755a..c6af0342 100644
--- a/dbus/dbus-connection.c
+++ b/dbus/dbus-connection.c
@@ -104,7 +104,7 @@ struct DBusConnection
 
   DBusHashTable *pending_replies;  /**< Hash of message serials and their message handlers. */  
   
-  int client_serial;            /**< Client serial. Increments each time a message is sent  */
+  dbus_uint32_t client_serial;       /**< Client serial. Increments each time a message is sent  */
   DBusList *disconnect_message_link; /**< Preallocated list node for queueing the disconnection message */
 
   DBusWakeupMainFunction wakeup_main_function; /**< Function to wake up the mainloop  */
@@ -728,7 +728,7 @@ _dbus_connection_new_for_transport (DBusTransport *transport)
   return NULL;
 }
 
-static dbus_int32_t
+static dbus_uint32_t
 _dbus_connection_get_next_client_serial (DBusConnection *connection)
 {
   int serial;
@@ -1147,9 +1147,9 @@ void
 dbus_connection_send_preallocated (DBusConnection       *connection,
                                    DBusPreallocatedSend *preallocated,
                                    DBusMessage          *message,
-                                   dbus_int32_t         *client_serial)
+                                   dbus_uint32_t        *client_serial)
 {
-  dbus_int32_t serial;
+  dbus_uint32_t serial;
   
   _dbus_assert (preallocated->connection == connection);
   _dbus_assert (dbus_message_get_name (message) != NULL);
@@ -1176,14 +1176,18 @@ dbus_connection_send_preallocated (DBusConnection       *connection,
                  connection,
                  connection->n_outgoing);
 
-  if (dbus_message_get_serial (message) == -1)
+  if (dbus_message_get_serial (message) == 0)
     {
       serial = _dbus_connection_get_next_client_serial (connection);
       _dbus_message_set_serial (message, serial);
+      if (client_serial)
+        *client_serial = serial;
+    }
+  else
+    {
+      if (client_serial)
+        *client_serial = dbus_message_get_serial (message);
     }
-  
-  if (client_serial)
-    *client_serial = dbus_message_get_serial (message);
   
   _dbus_message_lock (message);
 
@@ -1217,7 +1221,7 @@ dbus_connection_send_preallocated (DBusConnection       *connection,
 dbus_bool_t
 dbus_connection_send (DBusConnection *connection,
                       DBusMessage    *message,
-                      dbus_int32_t   *client_serial)
+                      dbus_uint32_t  *client_serial)
 {
   DBusPreallocatedSend *preallocated;
 
@@ -1379,7 +1383,7 @@ dbus_connection_send_with_reply (DBusConnection     *connection,
   data->connection_added = TRUE;
   
   /* Assign a serial to the message */
-  if (dbus_message_get_serial (message) == -1)
+  if (dbus_message_get_serial (message) == 0)
     {
       serial = _dbus_connection_get_next_client_serial (connection);
       _dbus_message_set_serial (message, serial);
@@ -1433,7 +1437,7 @@ dbus_connection_send_with_reply (DBusConnection     *connection,
 
 static DBusMessage*
 check_for_reply_unlocked (DBusConnection *connection,
-                          dbus_int32_t    client_serial)
+                          dbus_uint32_t   client_serial)
 {
   DBusList *link;
   
@@ -1483,7 +1487,7 @@ dbus_connection_send_with_reply_and_block (DBusConnection     *connection,
                                            int                 timeout_milliseconds,
                                            DBusError          *error)
 {
-  dbus_int32_t client_serial;
+  dbus_uint32_t client_serial;
   long start_tv_sec, start_tv_usec;
   long end_tv_sec, end_tv_usec;
   long tv_sec, tv_usec;
@@ -1520,7 +1524,7 @@ dbus_connection_send_with_reply_and_block (DBusConnection     *connection,
   end_tv_sec += end_tv_usec / _DBUS_USEC_PER_SECOND;
   end_tv_usec = end_tv_usec % _DBUS_USEC_PER_SECOND;
 
-  _dbus_verbose ("dbus_connection_send_with_reply_and_block(): will block %d milliseconds for reply serial %d from %ld sec %ld usec to %ld sec %ld usec\n",
+  _dbus_verbose ("dbus_connection_send_with_reply_and_block(): will block %d milliseconds for reply serial %u from %ld sec %ld usec to %ld sec %ld usec\n",
                  timeout_milliseconds,
                  client_serial,
                  start_tv_sec, start_tv_usec,
diff --git a/dbus/dbus-connection.h b/dbus/dbus-connection.h
index 5421e1f7..7e1de0cc 100644
--- a/dbus/dbus-connection.h
+++ b/dbus/dbus-connection.h
@@ -101,7 +101,7 @@ DBusDispatchStatus dbus_connection_get_dispatch_status          (DBusConnection
 DBusDispatchStatus dbus_connection_dispatch                     (DBusConnection             *connection);
 dbus_bool_t        dbus_connection_send                         (DBusConnection             *connection,
                                                                  DBusMessage                *message,
-                                                                 dbus_int32_t               *client_serial);
+                                                                 dbus_uint32_t              *client_serial);
 dbus_bool_t        dbus_connection_send_with_reply              (DBusConnection             *connection,
                                                                  DBusMessage                *message,
                                                                  DBusMessageHandler         *reply_handler,
@@ -198,7 +198,7 @@ void                  dbus_connection_free_preallocated_send (DBusConnection
 void                  dbus_connection_send_preallocated      (DBusConnection       *connection,
                                                               DBusPreallocatedSend *preallocated,
                                                               DBusMessage          *message,
-                                                              dbus_int32_t         *client_serial);
+                                                              dbus_uint32_t        *client_serial);
 
 
 DBUS_END_DECLS;
diff --git a/dbus/dbus-message.c b/dbus/dbus-message.c
index 97f8e5c5..fda50d6c 100644
--- a/dbus/dbus-message.c
+++ b/dbus/dbus-message.c
@@ -92,8 +92,8 @@ struct DBusMessage
                                            * of each field in "header"
                                            */
 
-  dbus_int32_t client_serial; /**< Cached client serial value for speed */
-  dbus_int32_t reply_serial;  /**< Cached reply serial value for speed */
+  dbus_uint32_t client_serial; /**< Cached client serial value for speed */
+  dbus_uint32_t reply_serial;  /**< Cached reply serial value for speed */
   
   int header_padding; /**< bytes of alignment in header */
   
@@ -204,6 +204,47 @@ adjust_field_offsets (DBusMessage *message,
     }
 }
 
+#ifdef DBUS_BUILD_TESTS
+/* tests-only until it's actually used */
+static dbus_int32_t
+get_int_field (DBusMessage *message,
+               int          field)
+{
+  int offset;
+
+  _dbus_assert (field < FIELD_LAST);
+  
+  offset = message->header_fields[field].offset;
+  
+  if (offset < 0)
+    return -1; /* useless if -1 is a valid value of course */
+  
+  return _dbus_demarshal_int32 (&message->header,
+                                message->byte_order,
+                                offset,
+                                NULL);
+}
+#endif
+
+static dbus_uint32_t
+get_uint_field (DBusMessage *message,
+                int          field)
+{
+  int offset;
+  
+  _dbus_assert (field < FIELD_LAST);
+  
+  offset = message->header_fields[field].offset;
+  
+  if (offset < 0)
+    return -1; /* useless if -1 is a valid value of course */
+  
+  return _dbus_demarshal_uint32 (&message->header,
+                                 message->byte_order,
+                                 offset,
+                                 NULL);
+}
+
 static const char*
 get_string_field (DBusMessage *message,
                   int          field,
@@ -235,25 +276,7 @@ get_string_field (DBusMessage *message,
   return data + (offset + 4); 
 }
 
-static dbus_int32_t
-get_int_field (DBusMessage *message,
-               int          field)
-{
-  int offset;
-
-  _dbus_assert (field < FIELD_LAST);
-  
-  offset = message->header_fields[field].offset;
-  
-  if (offset < 0)
-    return -1; /* useless if -1 is a valid value of course */
-  
-  return _dbus_demarshal_int32 (&message->header,
-                                message->byte_order,
-                                offset,
-                                NULL);
-}
-
+#ifdef DBUS_BUILD_TESTS
 static dbus_bool_t
 append_int_field (DBusMessage *message,
                   int          field,
@@ -303,6 +326,57 @@ append_int_field (DBusMessage *message,
     _dbus_assert_not_reached ("failed to reappend header padding");
   return FALSE;
 }
+#endif
+
+static dbus_bool_t
+append_uint_field (DBusMessage *message,
+                   int          field,
+                   const char  *name,
+                   int          value)
+{
+  int orig_len;
+
+  _dbus_assert (!message->locked);
+
+  clear_header_padding (message);
+  
+  orig_len = _dbus_string_get_length (&message->header);
+  
+  if (!_dbus_string_align_length (&message->header, 4))
+    goto failed;  
+  
+  if (!_dbus_string_append_len (&message->header, name, 4))
+    goto failed;
+
+  if (!_dbus_string_append_byte (&message->header, DBUS_TYPE_UINT32))
+    goto failed;
+
+  if (!_dbus_string_align_length (&message->header, 4))
+    goto failed;
+  
+  message->header_fields[FIELD_REPLY_SERIAL].offset =
+    _dbus_string_get_length (&message->header);
+  
+  if (!_dbus_marshal_uint32 (&message->header, message->byte_order,
+                             value))
+    goto failed;
+
+  if (!append_header_padding (message))
+    goto failed;
+  
+  return TRUE;
+  
+ failed:
+  message->header_fields[field].offset = -1;
+  _dbus_string_set_length (&message->header, orig_len);
+
+  /* this must succeed because it was allocated on function entry and
+   * DBusString doesn't ever realloc smaller
+   */
+  if (!append_header_padding (message))
+    _dbus_assert_not_reached ("failed to reappend header padding");
+  return FALSE;
+}
 
 static dbus_bool_t
 append_string_field (DBusMessage *message,
@@ -360,8 +434,8 @@ append_string_field (DBusMessage *message,
  * keep it compiling if we need it in future
  */
 static void
-delete_int_field (DBusMessage *message,
-                  int          field)
+delete_int_or_uint_field (DBusMessage *message,
+                          int          field)
 {
   int offset = message->header_fields[field].offset;
 
@@ -424,6 +498,7 @@ delete_string_field (DBusMessage *message,
   append_header_padding (message);
 }
 
+#ifdef DBUS_BUILD_TESTS
 static dbus_bool_t
 set_int_field (DBusMessage *message,
                int          field,
@@ -439,10 +514,6 @@ set_int_field (DBusMessage *message,
 
       switch (field)
         {
-        case FIELD_REPLY_SERIAL:
-          return append_int_field (message, field,
-                                   DBUS_HEADER_FIELD_REPLY,
-                                   value);
         default:
           _dbus_assert_not_reached ("appending an int field we don't support appending");
           return FALSE;
@@ -457,6 +528,7 @@ set_int_field (DBusMessage *message,
       return TRUE;
     }
 }
+#endif
 
 static dbus_bool_t
 set_uint_field (DBusMessage  *message,
@@ -473,6 +545,10 @@ set_uint_field (DBusMessage  *message,
 
       switch (field)
         {
+        case FIELD_REPLY_SERIAL:
+          return append_uint_field (message, field,
+                                    DBUS_HEADER_FIELD_REPLY,
+                                    value);
         default:
           _dbus_assert_not_reached ("appending a uint field we don't support appending");
           return FALSE;
@@ -568,10 +644,10 @@ _dbus_message_set_serial (DBusMessage  *message,
                           dbus_int32_t  serial)
 {
   _dbus_assert (!message->locked);
-  _dbus_assert (dbus_message_get_serial (message) < 0);
+  _dbus_assert (dbus_message_get_serial (message) == 0);
   
-  set_int_field (message, FIELD_CLIENT_SERIAL,
-                 serial);
+  set_uint_field (message, FIELD_CLIENT_SERIAL,
+                  serial);
   message->client_serial = serial;
 }
 
@@ -584,13 +660,13 @@ _dbus_message_set_serial (DBusMessage  *message,
  * @returns #FALSE if not enough memory
  */
 dbus_bool_t
-dbus_message_set_reply_serial (DBusMessage  *message,
-                               dbus_int32_t  reply_serial)
+dbus_message_set_reply_serial (DBusMessage   *message,
+                               dbus_uint32_t  reply_serial)
 {
   _dbus_assert (!message->locked);
 
-  if (set_int_field (message, FIELD_REPLY_SERIAL,
-                     reply_serial))
+  if (set_uint_field (message, FIELD_REPLY_SERIAL,
+                      reply_serial))
     {
       message->reply_serial = reply_serial;
       return TRUE;
@@ -607,7 +683,7 @@ dbus_message_set_reply_serial (DBusMessage  *message,
  * @param message the message
  * @returns the client serial
  */
-dbus_int32_t
+dbus_uint32_t
 dbus_message_get_serial (DBusMessage *message)
 {
   return message->client_serial;
@@ -615,12 +691,12 @@ dbus_message_get_serial (DBusMessage *message)
 
 /**
  * Returns the serial that the message is
- * a reply to or -1 if none.
+ * a reply to or 0 if none.
  *
  * @param message the message
  * @returns the reply serial
  */
-dbus_int32_t
+dbus_uint32_t
 dbus_message_get_reply_serial  (DBusMessage *message)
 {
   return message->reply_serial;
@@ -823,8 +899,8 @@ dbus_message_new_empty_header (void)
   
   message->refcount = 1;
   message->byte_order = DBUS_COMPILER_BYTE_ORDER;
-  message->client_serial = -1;
-  message->reply_serial = -1;
+  message->client_serial = 0;
+  message->reply_serial = 0;
   
   i = 0;
   while (i < FIELD_LAST)
@@ -3926,10 +4002,11 @@ _dbus_message_loader_queue_messages (DBusMessageLoader *loader)
           _dbus_assert (_dbus_string_get_length (&message->body) == body_len);
 
           /* Fill in caches */
-          message->reply_serial = get_int_field (message,
-                                                 FIELD_REPLY_SERIAL);
-          message->client_serial = get_int_field (message,
-                                                  FIELD_CLIENT_SERIAL);
+          /* FIXME there's no type check if the field has the wrong type */
+          message->reply_serial = get_uint_field (message,
+                                                  FIELD_REPLY_SERIAL);
+          message->client_serial = get_uint_field (message,
+                                                   FIELD_CLIENT_SERIAL);
           
 	  _dbus_verbose ("Loaded message %p\n", message);
 	}
@@ -4380,15 +4457,15 @@ check_message_handling (DBusMessage *message)
   DBusMessageIter iter;
   int type;
   dbus_bool_t retval;
-  dbus_int32_t client_serial;
+  dbus_uint32_t client_serial;
   
   retval = FALSE;
   
   client_serial = dbus_message_get_serial (message);
 
   /* can't use set_serial due to the assertions at the start of it */
-  set_int_field (message, FIELD_CLIENT_SERIAL,
-                 client_serial);
+  set_uint_field (message, FIELD_CLIENT_SERIAL,
+                  client_serial);
   
   if (client_serial != dbus_message_get_serial (message))
     {
diff --git a/dbus/dbus-message.h b/dbus/dbus-message.h
index 0ea03388..c692f919 100644
--- a/dbus/dbus-message.h
+++ b/dbus/dbus-message.h
@@ -63,28 +63,26 @@ DBusMessage* dbus_message_new_error_reply  (DBusMessage       *original_message,
 					    const char        *error_message);
 DBusMessage *dbus_message_copy             (const DBusMessage *message);
 
-void         dbus_message_ref   (DBusMessage *message);
-void         dbus_message_unref (DBusMessage *message);
-
-const char*  dbus_message_get_name         (DBusMessage  *message);
-const char*  dbus_message_get_service      (DBusMessage  *message);
-dbus_bool_t  dbus_message_set_sender       (DBusMessage  *message,
-                                            const char   *sender);
-const char*  dbus_message_get_sender       (DBusMessage  *message);
-void         dbus_message_set_is_error     (DBusMessage  *message,
-                                            dbus_bool_t   is_error_reply);
-dbus_bool_t  dbus_message_get_is_error     (DBusMessage  *message);
-dbus_bool_t  dbus_message_name_is          (DBusMessage  *message,
-                                            const char   *name);
-dbus_bool_t  dbus_message_service_is       (DBusMessage  *message,
-                                            const char   *service);
-dbus_bool_t  dbus_message_sender_is        (DBusMessage  *message,
-                                            const char   *service);
-dbus_int32_t dbus_message_get_serial       (DBusMessage  *message);
-dbus_bool_t  dbus_message_set_reply_serial (DBusMessage  *message,
-                                            dbus_int32_t  reply_serial);
-dbus_int32_t dbus_message_get_reply_serial (DBusMessage  *message);
-
+void          dbus_message_ref              (DBusMessage   *message);
+void          dbus_message_unref            (DBusMessage   *message);
+const char*   dbus_message_get_name         (DBusMessage   *message);
+const char*   dbus_message_get_service      (DBusMessage   *message);
+dbus_bool_t   dbus_message_set_sender       (DBusMessage   *message,
+                                             const char    *sender);
+const char*   dbus_message_get_sender       (DBusMessage   *message);
+void          dbus_message_set_is_error     (DBusMessage   *message,
+                                             dbus_bool_t    is_error_reply);
+dbus_bool_t   dbus_message_get_is_error     (DBusMessage   *message);
+dbus_bool_t   dbus_message_name_is          (DBusMessage   *message,
+                                             const char    *name);
+dbus_bool_t   dbus_message_service_is       (DBusMessage   *message,
+                                             const char    *service);
+dbus_bool_t   dbus_message_sender_is        (DBusMessage   *message,
+                                             const char    *service);
+dbus_uint32_t dbus_message_get_serial       (DBusMessage   *message);
+dbus_bool_t   dbus_message_set_reply_serial (DBusMessage   *message,
+                                             dbus_uint32_t  reply_serial);
+dbus_uint32_t dbus_message_get_reply_serial (DBusMessage   *message);
 
 dbus_bool_t dbus_message_append_args          (DBusMessage     *message,
 					       int              first_arg_type,
diff --git a/doc/dbus-specification.sgml b/doc/dbus-specification.sgml
index c14e9cdf..02feed3f 100644
--- a/doc/dbus-specification.sgml
+++ b/doc/dbus-specification.sgml
@@ -171,11 +171,10 @@
               </row>      
               <row>
                 <entry>4 bytes</entry>
-                <entry>The message's serial number, a signed 32-bit integer in
+                <entry>The message's serial number, an unsigned 32-bit integer in
                   the message's byte order. Applications MUST NOT reuse the same
                   serial number for different messages more often than 32-bit
-                  integer wraparound. Serial numbers must be greater than 
-                  zero.
+                  unsigned integer wraparound. Zero is not a valid serial number.
                 </entry>
               </row>
             </tbody>
@@ -247,7 +246,7 @@
               </row>
               <row>
                 <entry>rply</entry>
-                <entry>INT32</entry>
+                <entry>UINT32</entry>
                 <entry>The serial number of the message this message is a reply
                 to. (The serial number is one of the mandatory header fields,
                 see <xref linkend="message-protocol-header-encoding">.)</entry>