mds-colour: style + fix buffer overflow + send message id

Signed-off-by: Mattias Andrée <maandree@operamail.com>

2 files changed, 43 insertions, 46 deletions

diff --git a/src/mds-colour.c b/src/mds-colour.c
index fee5d92..a7cc7a7 100644
--- a/src/mds-colour.c
+++ b/src/mds-colour.c
@@ -550,7 +550,7 @@ int handle_list_colours(const char* recv_client_id, const char* recv_message_id,
 int handle_get_colour(const char* recv_client_id, const char* recv_message_id, const char* recv_name)
 {
   colour_t colour;
-  ssize_t length;
+  size_t length;
   char* temp;
   
   if (strequals(recv_client_id, "0:0"))
@@ -568,34 +568,36 @@ int handle_get_colour(const char* recv_client_id, const char* recv_message_id, c
       return 0;
     }
   
-  snprintf(NULL, 0,
-	  "To: %s\n"
-	  "In response to: %s\n"
-	  "Bytes: %i\n"
-	  "Red: %"PRIu64"\n"
-	  "Green; %"PRIu64"\n"
-	  "Blue: %"PRIu64"\n"
-	  "\n%zn",
-	  recv_client_id, recv_message_id, colour.bytes,
-	  colour.red, colour.green, colour.blue, &length);
+  length = sizeof("To: \n"
+		  "In response to: \n"
+		  "Message ID: \n"
+		  "Bytes: \n"
+		  "Red: \n"
+		  "Green; \n"
+		  "Blue: \n"
+		  "\n") / sizeof(char);
+  length += strlen(recv_client_id) + strlen(recv_message_id) + 10 + 1 + 3 * 3 * (size_t)(colour.bytes);
   
-  if ((size_t)length > send_buffer_size)
+  if (length > send_buffer_size)
     {
-      fail_if (yrealloc(temp, send_buffer, (size_t)length, char));
-      send_buffer_size = (size_t)length;
+      fail_if (yrealloc(temp, send_buffer, length, char));
+      send_buffer_size = length;
     }
   
   sprintf(send_buffer,
 	  "To: %s\n"
 	  "In response to: %s\n"
+	  "Message ID: %"PRIu32"\n"
 	  "Bytes: %i\n"
 	  "Red: %"PRIu64"\n"
 	  "Green; %"PRIu64"\n"
 	  "Blue: %"PRIu64"\n"
 	  "\n",
-	  recv_client_id, recv_message_id, colour.bytes,
+	  recv_client_id, recv_message_id, message_id, colour.bytes,
 	  colour.red, colour.green, colour.blue);
-
+  
+  message_id = message_id == UINT32_MAX ? 0 : (message_id + 1);
+  
   fail_if (full_send(send_buffer, (size_t)length));
   return 0;
  fail:
@@ -703,18 +705,18 @@ int set_colour(const char* name, const colour_t* colour)
   
   if (colour == NULL)
     {
-      /* We have been asked to remove the colour */
+      /* We have been asked to remove the colour. */
       
       /* If the colour does not exist, there is no
          point in removing it, and an event should
-         not be broadcasted */
+         not be broadcasted. */
       if (found == 0)
 	return 0;
       
-      /* Remove the colour */
+      /* Remove the colour. */
       colour_list_remove(&colours, name);
       
-      /* Broadcast update event */
+      /* Broadcast update event. */
       fail_if (broadcast_update("colour-removed", name, NULL, "yes"));
     }
   else
@@ -725,10 +727,10 @@ int set_colour(const char* name, const colour_t* colour)
          so we have to make a copy that will not disappear. */
       fail_if (xstrdup(name_, name));
       
-      /* Add or modify the colour */
+      /* Add or modify the colour. */
       fail_if (colour_list_put(&colours, name_, colour));
       
-      /* Broadcast update event */
+      /* Broadcast update event. */
       if (found == 0)
 	fail_if (broadcast_update("colour-added", name, colour, "yes"));
       else if (!colourequals(colour, &old_colour))
@@ -739,7 +741,7 @@ int set_colour(const char* name, const colour_t* colour)
  fail:
   saved_errno = errno;
   /* On failure, `colour_list_put` does not
-     free the colour name we gave it */
+     free the colour name we gave it. */
   free(name_);
   return errno = saved_errno, -1;
 }
@@ -748,40 +750,31 @@ int set_colour(const char* name, const colour_t* colour)
 /**
  * Broadcast a colour list update event
  * 
- * @param   event        The event, that is, the value for the Command-header, must not be `NULL`
+ * @param   event        The event, that is, the value for the `Command`-header, must not be `NULL`
  * @param   name         The name of the colour, must not be `NULL`
  * @param   colour       The new colour, `NULL` if and only if removed
- * @param   last_update  The value on the Last update-header
+ * @param   last_update  The value on the `Last update`-header
  * @return               Zero on success, -1 on error
  */
 int broadcast_update(const char* event, const char* name, const colour_t* colour, const char* last_update)
 {
   ssize_t part_length;
-  size_t length = 0;
+  size_t length;
   char* temp;
   
   free(colour_list_buffer_without_values), colour_list_buffer_without_values = NULL;
   free(colour_list_buffer_with_values),    colour_list_buffer_with_values = NULL;
   
-  snprintf(NULL, 0,
-	   "Command: %s\n"
-	   "Name: %s\n"
-	   "Last update: %s\n"
-	   "\n%zn",
-	   event, name, last_update, &part_length),
-    length += (size_t)part_length;
+  length = sizeof("Command: \nMessage ID: \nName: \nLast update: \n\n") / sizeof(char) - 1;
+  length += strlen(event) + 10 + strlen(name) + strlen(last_update);
   
   if (colour != NULL)
-    snprintf(NULL, 0,
-	     "Bytes: %i\n"
-	     "Red: %"PRIu64"\n"
-	     "Blue: %"PRIu64"\n"
-	     "Green: %"PRIu64"\n%zn",
-	     colour->bytes, colour->red, colour->green,
-	     colour->blue, &part_length),
-      length += (size_t)part_length;
+    {
+      length += sizeof("Bytes: \nRed: \nBlue: \nGreen: \n") / sizeof(char) - 1;
+      length += 1 + 3 * 3 * (size_t)(colour->bytes);
+    }
   
-  if (length > send_buffer_size)
+  if (++length > send_buffer_size)
     {
       fail_if (yrealloc(temp, send_buffer, length, char));
       send_buffer_size = length;
@@ -791,8 +784,9 @@ int broadcast_update(const char* event, const char* name, const colour_t* colour
   
   sprintf(send_buffer + length,
 	  "Command: %s\n"
+	  "Message ID: %"PRIu32"\n"
 	  "Name: %s\n%zn",
-	  event, name, &part_length),
+	  event, message_id, name, &part_length),
     length += (size_t)part_length;
   
   if (colour != NULL)
@@ -811,7 +805,10 @@ int broadcast_update(const char* event, const char* name, const colour_t* colour
 	  last_update, &part_length),
     length += (size_t)part_length;
   
-  return full_send(send_buffer, length);
+  message_id = message_id == UINT32_MAX ? 0 : (message_id + 1);
+  
+  fail_if (full_send(send_buffer, length));
+  return 0;
  fail:
   return -1;
 }
diff --git a/src/mds-colour.h b/src/mds-colour.h
index 341ffc8..703fe5e 100644
--- a/src/mds-colour.h
+++ b/src/mds-colour.h
@@ -115,10 +115,10 @@ int set_colour(const char* name, const colour_t* colour) __attribute__((nonnull(
 /**
  * Broadcast a colour list update event
  * 
- * @param   event        The event, that is, the value for the Command-header, must not be `NULL`
+ * @param   event        The event, that is, the value for the `Command`-header, must not be `NULL`
  * @param   name         The name of the colour, must not be `NULL`
  * @param   colour       The new colour, `NULL` if and only if removed
- * @param   last_update  The value on the Last update-header
+ * @param   last_update  The value on the `Last update`-header
  * @return               Zero on success, -1 on error
  */
 int broadcast_update(const char* event, const char* name, const colour_t* colour,