with a few exceptions and some remaining files, never return directly on failure, always goto fail by invoking fail_if

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

5 files changed, 77 insertions, 83 deletions

diff --git a/src/mds-registry/mds-registry.c b/src/mds-registry/mds-registry.c
index 44cbe86..4b49525 100644
--- a/src/mds-registry/mds-registry.c
+++ b/src/mds-registry/mds-registry.c
@@ -82,6 +82,7 @@ int preinitialise_server(void)
  */
 int initialise_server(void)
 {
+  int stage = 0;
   const char* const message =
     "Command: intercept\n"
     "Message ID: 0\n"
@@ -103,14 +104,8 @@ int initialise_server(void)
         that happen between the crash and the recovery.
    */
   
-  if (full_send(message, strlen(message)))
-    return 1;
-  if (hash_table_create_tuned(&reg_table, 32))
-    {
-      xperror(*argv);
-      hash_table_destroy(&reg_table, NULL, NULL);
-      return 1;
-    }
+  fail_if (full_send(message, strlen(message)));  stage++;
+  fail_if (hash_table_create_tuned(&reg_table, 32));  stage++;
   reg_table.key_comparator = (compare_func*)string_comparator;
   reg_table.hasher = (hash_func*)string_hash;
   fail_if (server_initialised() < 0);
@@ -119,7 +114,10 @@ int initialise_server(void)
   return 0;  
  fail:
   xperror(*argv);
-  mds_message_destroy(&received);
+  if (stage == 1)
+    hash_table_destroy(&reg_table, NULL, NULL);
+  if (stage == 2)
+    mds_message_destroy(&received);
   return 1;
 }
 
@@ -135,13 +133,12 @@ int postinitialise_server(void)
   if (connected)
     return 0;
   
-  if (reconnect_to_display())
-    {
-      mds_message_destroy(&received);
-      return 1;
-    }
+  fail_if (reconnect_to_display());
   connected = 1;
   return 0;
+ fail:
+  mds_message_destroy(&received);
+  return 1;
 }
 
 
diff --git a/src/mds-registry/registry.c b/src/mds-registry/registry.c
index 5f241ea..f853f93 100644
--- a/src/mds-registry/registry.c
+++ b/src/mds-registry/registry.c
@@ -219,15 +219,14 @@ static int registry_action_act(char* command, int action, uint64_t client, hash_
   if (action == 1)
     {
       /* Register server to protocol. */
-      if (registry_action_add(has_key, command, command_key, client))
-	return -1;
+      fail_if (registry_action_add(has_key, command, command_key, client));
     }
   else if ((action == -1) && has_key)
     /* Unregister server from protocol. */
     registry_action_remove(command_key, client);
   else if ((action == 0) && !has_key)
     {
-      /* Add protocl to wait set of not present in the protocol table. */
+      /* Add protocol to wait set of not present in the protocol table. */
       fail_if ((command = strdup(command)) == NULL);
       command_key = (size_t)(void*)command;
       if (hash_table_put(wait_set, command_key, 1) == 0)
@@ -241,8 +240,8 @@ static int registry_action_act(char* command, int action, uint64_t client, hash_
   return 0;
  fail:
   xperror(*argv);
-  hash_table_destroy(wait_set, (free_func*)reg_table_free_key, NULL);
-  free(wait_set);
+  if (action != 1)
+    hash_table_destroy(wait_set, (free_func*)reg_table_free_key, NULL), free(wait_set);
   return -1;
 }
 
@@ -264,21 +263,15 @@ static int registry_action(size_t length, int action, const char* recv_client_id
   uint64_t client = action ? parse_client_id(recv_client_id) : 0;
   hash_table_t* wait_set = NULL;
   size_t begin;
+  int saved_errno;
   
   
   /* If ‘Action: wait’, create a set for the protocols that are not already available. */
   
   if (action == 0)
     {
-      wait_set = malloc(sizeof(hash_table_t));
-      if (wait_set == NULL)
-	return -1;
-      if (hash_table_create(wait_set))
-	{
-	  hash_table_destroy(wait_set, NULL, NULL);
-	  free(wait_set);
-	  return -1;
-	}
+      fail_if (xmalloc(wait_set, 1, hash_table_t));
+      fail_if (hash_table_create(wait_set));
       wait_set->key_comparator = (compare_func*)string_comparator;
       wait_set->hasher = (hash_func*)string_hash;
     }
@@ -288,14 +281,8 @@ static int registry_action(size_t length, int action, const char* recv_client_id
   
   if (received.payload_size == length)
     {
-      if (growalloc(old, received.payload, received.payload_size, char))
-	{
-	  if (wait_set != NULL)
-	    hash_table_destroy(wait_set, NULL, NULL), free(wait_set);
-	  return -1;
-	}
-      else
-	payload = received.payload;
+      fail_if (growalloc(old, received.payload, received.payload_size, char));
+      payload = received.payload;
     }
   
   
@@ -318,16 +305,22 @@ static int registry_action(size_t length, int action, const char* recv_client_id
       
       if (len > 0)
 	if (registry_action_act(command, action, client, wait_set))
-	  return -1;
+	  fail_if (wait_set = NULL, 1);
     }
   
   
   /* If ‘Action: wait’, start a new thread that waits for the protocols and the responds. */
   
   if (action == 0)
-    return start_slave(wait_set, recv_client_id, recv_message_id);
+    if (start_slave(wait_set, recv_client_id, recv_message_id))
+      fail_if (wait_set = NULL, 1);
   
   return 0;
+ fail:
+  saved_errno = errno;
+  if (wait_set != NULL)
+    hash_table_destroy(wait_set, NULL, NULL), free(wait_set);
+  return errno = saved_errno, -1;
 }
 
 
@@ -336,7 +329,7 @@ static int registry_action(size_t length, int action, const char* recv_client_id
  * 
  * @param   recv_client_id   The ID of the client
  * @param   recv_message_id  The ID of the received message
- * @return                   Zero on success -1 on error or interruption,
+ * @return                   Zero on success, -1 on error or interruption,
  *                           `errno` will be set accordingly
  */
 static int list_registry(const char* recv_client_id, const char* recv_message_id)
@@ -349,8 +342,7 @@ static int list_registry(const char* recv_client_id, const char* recv_message_id
   
   if (send_buffer_size == 0)
     {
-      if (xmalloc(send_buffer, 256, char))
-	return -1;
+      fail_if (xmalloc(send_buffer, 256, char));
       send_buffer_size = 256;
     }
   
@@ -365,8 +357,7 @@ static int list_registry(const char* recv_client_id, const char* recv_message_id
       
       /* Make sure the send buffer can fit all protocols. */
       while (ptr + len + 1 >= send_buffer_size)
-	if (growalloc(old, send_buffer, send_buffer_size, char))
-	  return -1;
+	fail_if (growalloc(old, send_buffer, send_buffer_size, char));
       
       memcpy(send_buffer + ptr, command, len * sizeof(char));
       ptr += len;
@@ -380,8 +371,7 @@ static int list_registry(const char* recv_client_id, const char* recv_message_id
   i += strlen("To: %s\nIn response to: %s\nMessage ID: %" PRIu32 "\nLength: %" PRIu64 "\n\n");
   
   while (ptr + i >= send_buffer_size)
-    if (growalloc(old, send_buffer, send_buffer_size, char))
-      return -1;
+    fail_if (growalloc(old, send_buffer, send_buffer_size, char));
   
   
   /* Construct message headers. */
@@ -392,9 +382,10 @@ static int list_registry(const char* recv_client_id, const char* recv_message_id
   with_mutex (slave_mutex, message_id = message_id == UINT32_MAX ? 0 : (message_id + 1););
   
   /* Send message. */
-  if (full_send(send_buffer + ptr, strlen(send_buffer + ptr)))
-    return 1;
+  fail_if (full_send(send_buffer + ptr, strlen(send_buffer + ptr)));
   return full_send(send_buffer, ptr);
+ fail:
+  return -1;
 }
 
 
@@ -488,7 +479,7 @@ static int handle_register_message(void)
 /**
  * Handle the received message
  * 
- * @return  Zero on success -1 on error or interruption,
+ * @return  Zero on success, -1 on error or interruption,
  *          `errno` will be set accordingly
  */
 int handle_message(void)
@@ -496,7 +487,13 @@ int handle_message(void)
   size_t i;
   for (i = 0; i < received.header_count; i++)
     if (strequals(received.headers[i], "Command: register"))
-      return handle_register_message();
-  return handle_close_message();
+      {
+	fail_if (handle_register_message());
+	return 0;
+      }
+  fail_if (handle_close_message());
+  return 0;
+ fail:
+  return -1;
 }
 
diff --git a/src/mds-registry/registry.h b/src/mds-registry/registry.h
index 0de4d6f..f01b383 100644
--- a/src/mds-registry/registry.h
+++ b/src/mds-registry/registry.h
@@ -22,7 +22,7 @@
 /**
  * Handle the received message
  * 
- * @return  Zero on success -1 on error or interruption,
+ * @return  Zero on success, -1 on error or interruption,
  *          `errno` will be set accordingly
  */
 int handle_message(void);
diff --git a/src/mds-registry/slave.c b/src/mds-registry/slave.c
index 8a4d221..ef46963 100644
--- a/src/mds-registry/slave.c
+++ b/src/mds-registry/slave.c
@@ -55,13 +55,14 @@ static int slave_notify_client(slave_t* slave)
   while (left > 0)
     {
       sent = send_message(socket_fd, buf + ptr, left);
-      if ((sent < left) && errno && (errno != EINTR))
-	return -1;
+      fail_if ((sent < left) && errno && (errno != EINTR));
       left -= sent;
       ptr += sent;
     }
   
   return 0;
+ fail:
+  return -1;
 }
 
 
@@ -136,14 +137,12 @@ static void* slave_loop(void* data)
  */
 int start_created_slave(slave_t* restrict slave)
 {
-  if ((errno = pthread_mutex_lock(&slave_mutex)))
-    return -1;
+  int locked = 0;
   
-  if ((errno = pthread_create(&(slave->thread), NULL, slave_loop, (void*)(intptr_t)slave)))
-    {
-      pthread_mutex_unlock(&slave_mutex);
-      return -1;
-    }
+  fail_if ((errno = pthread_mutex_lock(&slave_mutex)));
+  locked = 1;
+  
+  fail_if ((errno = pthread_create(&(slave->thread), NULL, slave_loop, (void*)(intptr_t)slave)));
   
   if ((errno = pthread_detach(slave->thread)))
     xperror(*argv);
@@ -152,6 +151,10 @@ int start_created_slave(slave_t* restrict slave)
   pthread_mutex_unlock(&slave_mutex);
   
   return 0;
+ fail:
+  if (locked)
+    pthread_mutex_unlock(&slave_mutex);
+  return -1;
 }
 
 
@@ -239,8 +242,7 @@ int advance_slaves(char* command)
   int signal_slaves = 0;
   ssize_t node;
   
-  if ((errno = pthread_mutex_lock(&slave_mutex)))
-    return -1;
+  fail_if ((errno = pthread_mutex_lock(&slave_mutex)));
   
   foreach_linked_list_node (slave_list, node)
     {
@@ -257,6 +259,8 @@ int advance_slaves(char* command)
 
   pthread_mutex_unlock(&slave_mutex);
   return 0;
+ fail:
+  return -1;
 }
 
 
@@ -410,7 +414,8 @@ size_t slave_marshal(const slave_t* restrict this, char* restrict data)
 size_t slave_unmarshal(slave_t* restrict this, char* restrict data)
 {
   size_t key, n, m, rc = 2 * sizeof(int) + sizeof(ssize_t) + sizeof(size_t) + sizeof(uint64_t);
-  char* protocol;
+  char* protocol = NULL;
+  int saved_errno;
   
   this->wait_set = NULL;
   this->client_id = NULL;
@@ -427,42 +432,37 @@ size_t slave_unmarshal(slave_t* restrict this, char* restrict data)
   buf_get_next(data, long, this->dethklok.tv_nsec);
   
   n = (strlen((char*)data) + 1) * sizeof(char);
-  if ((this->client_id = malloc(n)) == NULL)
-    return 0;
+  fail_if ((this->client_id = malloc(n)) == NULL);
   memcpy(this->client_id, data, n);
   data += n, rc += n;
   
   n = (strlen((char*)data) + 1) * sizeof(char);
-  if ((this->message_id = malloc(n)) == NULL)
-    return 0;
+  fail_if ((this->message_id = malloc(n)) == NULL);
   memcpy(this->message_id, data, n);
   data += n, rc += n;
   
-  if ((this->wait_set = malloc(sizeof(hash_table_t))) == NULL)
-    return 0;
-  if (hash_table_create(this->wait_set))
-    return 0;
+  fail_if ((this->wait_set = malloc(sizeof(hash_table_t))) == NULL);
+  fail_if (hash_table_create(this->wait_set));
   
   buf_get_next(data, size_t, m);
   
   while (m--)
     {
       n = (strlen((char*)data) + 1) * sizeof(char);
-      if ((protocol = malloc(n)) == NULL)
-	return 0;
+      fail_if ((protocol = malloc(n)) == NULL);
       memcpy(protocol, data, n);
       data += n, rc += n;
       
       key = (size_t)(void*)protocol;
       if (hash_table_put(this->wait_set, key, 1) == 0)
-	if (errno)
-	  {
-	    free(protocol);
-	    return 0;
-	  }
+	fail_if (errno);
     }
   
   return rc;
+ fail:
+  saved_errno = errno;
+  free(protocol);
+  return errno = saved_errno, (size_t)0;
 }
 
 
diff --git a/src/mds-registry/util.c b/src/mds-registry/util.c
index 1963eeb..6b383c2 100644
--- a/src/mds-registry/util.c
+++ b/src/mds-registry/util.c
@@ -76,14 +76,14 @@ int full_send(const char* message, size_t length)
 	  eprint("Sent more of a message than exists in the message, aborting.");
 	  return -1;
 	}
-      else if ((sent < length) && (errno != EINTR))
-	{
-	  xperror(*argv);
-	  return -1;
-	}
+      else
+	fail_if ((sent < length) && (errno != EINTR));
       message += sent;
       length -= sent;
     }
   return 0;
+ fail:
+  xperror(*argv);
+  return -1;
 }