Make error codes returned by librabbitmq functions opaque

Windows doesn't generally use POSIX error codes, which poses a problem
for librabbitmq's approach of using those error codes in its API.  So
make the librabbitmq error codes opaque: They are still be integers,
but client code is not supposed to assume anything about them, except
that they can be passed to a new amqp_error_string() function which
returns the corresponding error message

Internally, the error codes are either taken from a set of
librabbitmq-specific values, or correspond to an OS-specific (POSIX or
win32) error code, with a simple encoding to indicate which is which.

8 files changed, 133 insertions, 70 deletions

diff --git a/librabbitmq/amqp.h b/librabbitmq/amqp.h
index b67d6c8..58864e9 100644
--- a/librabbitmq/amqp.h
+++ b/librabbitmq/amqp.h
@@ -263,7 +263,7 @@ typedef enum amqp_response_type_enum_ {
 typedef struct amqp_rpc_reply_t_ {
   amqp_response_type_enum reply_type;
   amqp_method_t reply;
-  int library_errno; /* if AMQP_RESPONSE_LIBRARY_EXCEPTION, then 0 here means socket EOF */
+  int library_error; /* if AMQP_RESPONSE_LIBRARY_EXCEPTION, then 0 here means socket EOF */
 } amqp_rpc_reply_t;
 
 typedef enum amqp_sasl_method_enum_ {
@@ -495,6 +495,14 @@ extern amqp_boolean_t amqp_data_in_buffer(amqp_connection_state_t state);
  */
 extern amqp_rpc_reply_t amqp_get_rpc_reply(amqp_connection_state_t state);
 
+/*
+ * Get the error string for the given error code.
+ *
+ * The returned string resides on the heap; the caller is responsible
+ * for freeing it.
+ */
+extern const char *amqp_error_string(int err);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/librabbitmq/amqp_api.c b/librabbitmq/amqp_api.c
index 91b0bf8..3a4440f 100644
--- a/librabbitmq/amqp_api.c
+++ b/librabbitmq/amqp_api.c
@@ -60,6 +60,41 @@
 
 #include <assert.h>
 
+static const char *client_error_strings[ERROR_MAX] = {
+  "could not allocate memory", /* ERROR_NO_MEMORY */
+  "received bad AMQP data", /* ERROR_BAD_AQMP_DATA */
+  "unknown AMQP class id", /* ERROR_UNKOWN_CLASS */
+  "unknown AMQP method id", /* ERROR_UNKOWN_METHOD */
+  "unknown host", /* ERROR_HOST_NOT_FOUND */
+  "incompatible AMQP version", /* ERROR_INCOMPATIBLE_AMQP_VERSION */
+  "connection closed unexpectedly", /* ERROR_CONNECTION_CLOSED */
+};
+
+const char *amqp_error_string(int err)
+{
+  const char *str;
+  int category = (err & ERROR_CATEGORY_MASK);
+  err = (err & ~ERROR_CATEGORY_MASK);
+
+  switch (category) {
+  case ERROR_CATEGORY_CLIENT:
+    if (err < 1 || err > ERROR_MAX)
+      str = "(undefined librabbitmq error)";
+    else
+      str = client_error_strings[err - 1];
+    break;
+
+  case ERROR_CATEGORY_OS:
+    str = strerror(err);
+    break;
+
+  default:
+    str = "(undefined error category)";
+  }
+
+  return strdup(str);
+}
+
 #define RPC_REPLY(replytype)						\
   (state->most_recent_api_result.reply_type == AMQP_RESPONSE_NORMAL	\
    ? (replytype *) state->most_recent_api_result.reply.decoded		\
diff --git a/librabbitmq/amqp_connection.c b/librabbitmq/amqp_connection.c
index 7531fac..4fdc71e 100644
--- a/librabbitmq/amqp_connection.c
+++ b/librabbitmq/amqp_connection.c
@@ -151,7 +151,7 @@ int amqp_tune_connection(amqp_connection_state_t state,
   newbuf = realloc(state->outbound_buffer.bytes, frame_max);
   if (newbuf == NULL) {
     amqp_destroy_connection(state);
-    return -ENOMEM;
+    return -ERROR_NO_MEMORY;
   }
   state->outbound_buffer.bytes = newbuf;
 
@@ -174,7 +174,7 @@ int amqp_end_connection(amqp_connection_state_t state) {
   int s = state->sockfd;
   amqp_destroy_connection(state);
   if (close(s) < 0)
-    return -errno;
+    return -encoded_errno();
   else
     return 0;
 } 
@@ -208,7 +208,7 @@ int amqp_handle_input(amqp_connection_state_t state,
       /* state->inbound_buffer.len is always nonzero, because it
 	 corresponds to frame_max, which is not permitted to be less
 	 than AMQP_FRAME_MIN_SIZE (currently 4096 bytes). */
-      return -ENOMEM;
+      return -ERROR_NO_MEMORY;
     }
     state->state = CONNECTION_STATE_WAITING_FOR_HEADER;
   }
@@ -255,7 +255,7 @@ int amqp_handle_input(amqp_connection_state_t state,
 
       /* Check frame end marker (footer) */
       if (D_8(state->inbound_buffer, state->target_size - 1) != AMQP_FRAME_END) {
-	return -EINVAL;
+	return -ERROR_BAD_AMQP_DATA;
       }
 
       decoded_frame->channel = D_16(state->inbound_buffer, 1);
@@ -401,7 +401,7 @@ static int inner_send_frame(amqp_connection_state_t state,
       break;
 
     default:
-      return -EINVAL;
+      abort();
   }
 
   E_32(state->outbound_buffer, 3, *payload_len);
diff --git a/librabbitmq/amqp_mem.c b/librabbitmq/amqp_mem.c
index 6e52dc8..7783cbb 100644
--- a/librabbitmq/amqp_mem.c
+++ b/librabbitmq/amqp_mem.c
@@ -102,25 +102,24 @@ void empty_amqp_pool(amqp_pool_t *pool) {
   empty_blocklist(&pool->pages);
 }
 
+/* Returns 1 on success, 0 on failure */
 static int record_pool_block(amqp_pool_blocklist_t *x, void *block) {
   size_t blocklistlength = sizeof(void *) * (x->num_blocks + 1);
 
   if (x->blocklist == NULL) {
     x->blocklist = malloc(blocklistlength);
-    if (x->blocklist == NULL) {
-      return -ENOMEM;
-    }
+    if (x->blocklist == NULL)
+      return 0;
   } else {
     void *newbl = realloc(x->blocklist, blocklistlength);
-    if (newbl == NULL) {
-      return -ENOMEM;
-    }
+    if (newbl == NULL)
+      return 0;
     x->blocklist = newbl;
   }
 
   x->blocklist[x->num_blocks] = block;
   x->num_blocks++;
-  return 0;
+  return 1;
 }
 
 void *amqp_pool_alloc(amqp_pool_t *pool, size_t amount) {
@@ -135,9 +134,8 @@ void *amqp_pool_alloc(amqp_pool_t *pool, size_t amount) {
     if (result == NULL) {
       return NULL;
     }
-    if (record_pool_block(&pool->large_blocks, result) != 0) {
+    if (!record_pool_block(&pool->large_blocks, result))
       return NULL;
-    }
     return result;
   }
 
@@ -156,9 +154,8 @@ void *amqp_pool_alloc(amqp_pool_t *pool, size_t amount) {
     if (pool->alloc_block == NULL) {
       return NULL;
     }
-    if (record_pool_block(&pool->pages, pool->alloc_block) != 0) {
+    if (!record_pool_block(&pool->pages, pool->alloc_block))
       return NULL;
-    }
     pool->next_page = pool->pages.num_blocks;
   } else {
     pool->alloc_block = pool->pages.blocklist[pool->next_page];
diff --git a/librabbitmq/amqp_private.h b/librabbitmq/amqp_private.h
index 3985619..f922933 100644
--- a/librabbitmq/amqp_private.h
+++ b/librabbitmq/amqp_private.h
@@ -57,6 +57,30 @@ extern "C" {
 
 #include <arpa/inet.h> /* ntohl, htonl, ntohs, htons */
 
+/* Error numbering: Because of differences in error numbering on
+ * different platforms, we want to keep error numbers opaque for
+ * client code.  Internally, we encode the category of an error
+ * (i.e. where its number comes from) in the top bits of the number
+ * (assuming that an int has at least 32 bits).
+ */
+#define ERROR_CATEGORY_MASK (1 << 29)
+
+#define ERROR_CATEGORY_CLIENT (0 << 29) /* librabbitmq error codes */
+#define ERROR_CATEGORY_OS (1 << 29) /* OS-specific error codes */
+
+/* librabbitmq error codes */
+#define ERROR_NO_MEMORY 1
+#define ERROR_BAD_AMQP_DATA 2
+#define ERROR_UNKNOWN_CLASS 3
+#define ERROR_UNKNOWN_METHOD 4
+#define ERROR_HOST_NOT_FOUND 5
+#define ERROR_INCOMPATIBLE_AMQP_VERSION 6
+#define ERROR_CONNECTION_CLOSED 7
+#define ERROR_MAX 7
+
+/* Get the encoded form of errno */
+#define encoded_errno() (errno | ERROR_CATEGORY_OS)
+
 /*
  * Connection states:
  *
@@ -125,7 +149,7 @@ struct amqp_connection_state_t_ {
   amqp_rpc_reply_t most_recent_api_result;
 };
 
-#define CHECK_LIMIT(b, o, l, v) ({ if ((o + l) > (b).len) { return -EFAULT; } (v); })
+#define CHECK_LIMIT(b, o, l, v) ({ if ((o + l) > (b).len) { return -ERROR_BAD_AMQP_DATA; } (v); })
 #define BUF_AT(b, o) (&(((uint8_t *) (b).bytes)[o]))
 
 #define D_8(b, o) CHECK_LIMIT(b, o, 1, * (uint8_t *) BUF_AT(b, o))
@@ -176,13 +200,6 @@ extern int amqp_encode_table(amqp_bytes_t encoded,
 
 #define AMQP_CHECK_RESULT(expr) AMQP_CHECK_RESULT_CLEANUP(expr, )
 
-#define AMQP_CHECK_EOF_RESULT(expr)		\
-  ({						\
-    int _result = (expr);			\
-    if (_result <= 0) return _result;		\
-    _result;					\
-  })
-
 #ifndef NDEBUG
 extern void amqp_dump(void const *buffer, size_t len);
 #else
diff --git a/librabbitmq/amqp_socket.c b/librabbitmq/amqp_socket.c
index d16c319..8f3f05b 100644
--- a/librabbitmq/amqp_socket.c
+++ b/librabbitmq/amqp_socket.c
@@ -76,9 +76,8 @@ int amqp_open_socket(char const *hostname,
   struct hostent *he;
 
   he = gethostbyname(hostname);
-  if (he == NULL) {
-    return -ENOENT;
-  }
+  if (he == NULL)
+    return -ERROR_HOST_NOT_FOUND;
 
   addr.sin_family = AF_INET;
   addr.sin_port = htons(portnumber);
@@ -86,7 +85,7 @@ int amqp_open_socket(char const *hostname,
 
   sockfd = socket(PF_INET, SOCK_STREAM, 0);
   if (connect(sockfd, (struct sockaddr *) &addr, sizeof(addr)) < 0) {
-    int result = -errno;
+    int result = -encoded_errno();
     close(sockfd);
     return result;
   }
@@ -183,10 +182,9 @@ static int wait_frame_inner(amqp_connection_state_t state,
       AMQP_CHECK_RESULT((result = amqp_handle_input(state, buffer, decoded_frame)));
       state->sock_inbound_offset += result;
 
-      if (decoded_frame->frame_type != 0) {
+      if (decoded_frame->frame_type != 0)
 	/* Complete frame was read. Return it. */
-	return 1;
-      }
+	return 0;
 
       /* Incomplete or ignored frame. Keep processing input. */
       assert(result != 0);
@@ -195,12 +193,11 @@ static int wait_frame_inner(amqp_connection_state_t state,
     result = read(state->sockfd,
 		  state->sock_inbound_buffer.bytes,
 		  state->sock_inbound_buffer.len);
-    if (result < 0) {
-      return -errno;
-    }
-    if (result == 0) {
-      /* EOF. */
-      return 0;
+    if (result <= 0) {
+      if (result == 0)
+	return -ERROR_CONNECTION_CLOSED;
+      else
+	return -encoded_errno();
     }
 
     state->sock_inbound_limit = result;
@@ -218,7 +215,7 @@ int amqp_simple_wait_frame(amqp_connection_state_t state,
       state->last_queued_frame = NULL;
     }
     *decoded_frame = *f;
-    return 1;
+    return 0;
   } else {
     return wait_frame_inner(state, decoded_frame);
   }
@@ -230,8 +227,10 @@ int amqp_simple_wait_method(amqp_connection_state_t state,
 			    amqp_method_t *output)
 {
   amqp_frame_t frame;
-
-  AMQP_CHECK_EOF_RESULT(amqp_simple_wait_frame(state, &frame));
+  int res = amqp_simple_wait_frame(state, &frame);
+  if (res < 0)
+    return res;
+  
   amqp_assert(frame.channel == expected_channel,
 	      "Expected 0x%08X method frame on channel %d, got frame on channel %d",
 	      expected_method,
@@ -248,7 +247,7 @@ int amqp_simple_wait_method(amqp_connection_state_t state,
 	      expected_channel,
 	      frame.payload.method.id);
   *output = frame.payload.method;
-  return 1;
+  return 0;
 }
 
 int amqp_send_method(amqp_connection_state_t state,
@@ -288,7 +287,7 @@ amqp_rpc_reply_t amqp_simple_rpc(amqp_connection_state_t state,
   status = amqp_send_method(state, channel, request_id, decoded_request_method);
   if (status < 0) {
     result.reply_type = AMQP_RESPONSE_LIBRARY_EXCEPTION;
-    result.library_errno = -status;
+    result.library_error = -status;
     return result;
   }
 
@@ -297,9 +296,9 @@ amqp_rpc_reply_t amqp_simple_rpc(amqp_connection_state_t state,
 
   retry:
     status = wait_frame_inner(state, &frame);
-    if (status <= 0) {
+    if (status < 0) {
       result.reply_type = AMQP_RESPONSE_LIBRARY_EXCEPTION;
-      result.library_errno = -status;
+      result.library_error = -status;
       return result;
     }
 
@@ -324,7 +323,7 @@ amqp_rpc_reply_t amqp_simple_rpc(amqp_connection_state_t state,
 
       if (frame_copy == NULL || link == NULL) {
 	result.reply_type = AMQP_RESPONSE_LIBRARY_EXCEPTION;
-	result.library_errno = ENOMEM;
+	result.library_error = ERROR_NO_MEMORY;
 	return result;
       }
 
@@ -359,6 +358,7 @@ static int amqp_login_inner(amqp_connection_state_t state,
 			    amqp_sasl_method_enum sasl_method,
 			    va_list vl)
 {
+  int res;
   amqp_method_t method;
   uint32_t server_frame_max;
   uint16_t server_channel_max;
@@ -366,12 +366,16 @@ static int amqp_login_inner(amqp_connection_state_t state,
 
   amqp_send_header(state);
 
-  AMQP_CHECK_EOF_RESULT(amqp_simple_wait_method(state, 0, AMQP_CONNECTION_START_METHOD, &method));
+  res = amqp_simple_wait_method(state, 0, AMQP_CONNECTION_START_METHOD,
+				&method);
+  if (res < 0)
+    return res;
+
   {
     amqp_connection_start_t *s = (amqp_connection_start_t *) method.decoded;
     if ((s->version_major != AMQP_PROTOCOL_VERSION_MAJOR) ||
 	(s->version_minor != AMQP_PROTOCOL_VERSION_MINOR)) {
-      return -EPROTOTYPE;
+      return -ERROR_INCOMPATIBLE_AMQP_VERSION;
     }
 
     /* TODO: check that our chosen SASL mechanism is in the list of
@@ -383,7 +387,7 @@ static int amqp_login_inner(amqp_connection_state_t state,
     amqp_bytes_t response_bytes = sasl_response(&state->decoding_pool, sasl_method, vl);
     amqp_connection_start_ok_t s;
     if (response_bytes.bytes == NULL) {
-      return -ENOMEM;
+      return -ERROR_NO_MEMORY;
     }
     s =
       (amqp_connection_start_ok_t) {
@@ -397,7 +401,11 @@ static int amqp_login_inner(amqp_connection_state_t state,
 
   amqp_release_buffers(state);
 
-  AMQP_CHECK_EOF_RESULT(amqp_simple_wait_method(state, 0, AMQP_CONNECTION_TUNE_METHOD, &method));
+  res = amqp_simple_wait_method(state, 0, AMQP_CONNECTION_TUNE_METHOD,
+				&method);
+  if (res < 0)
+    return res;
+  
   {
     amqp_connection_tune_t *s = (amqp_connection_tune_t *) method.decoded;
     server_channel_max = s->channel_max;
@@ -431,7 +439,7 @@ static int amqp_login_inner(amqp_connection_state_t state,
 
   amqp_release_buffers(state);
 
-  return 1;
+  return 0;
 }
 
 amqp_rpc_reply_t amqp_login(amqp_connection_state_t state,
@@ -449,11 +457,11 @@ amqp_rpc_reply_t amqp_login(amqp_connection_state_t state,
   va_start(vl, sasl_method);
 
   status = amqp_login_inner(state, channel_max, frame_max, heartbeat, sasl_method, vl);
-  if (status <= 0) {
+  if (status < 0) {
     result.reply_type = AMQP_RESPONSE_LIBRARY_EXCEPTION;
     result.reply.id = 0;
     result.reply.decoded = NULL;
-    result.library_errno = -status;
+    result.library_error = -status;
     return result;
   }
 
@@ -481,6 +489,6 @@ amqp_rpc_reply_t amqp_login(amqp_connection_state_t state,
   result.reply_type = AMQP_RESPONSE_NORMAL;
   result.reply.id = 0;
   result.reply.decoded = NULL;
-  result.library_errno = 0;
+  result.library_error = 0;
   return result;
 }
diff --git a/librabbitmq/amqp_table.c b/librabbitmq/amqp_table.c
index 25c5932..db45588 100644
--- a/librabbitmq/amqp_table.c
+++ b/librabbitmq/amqp_table.c
@@ -86,7 +86,7 @@ static int amqp_decode_array(amqp_bytes_t encoded,
   int limit;
 
   if (entries == NULL) {
-    return -ENOMEM;
+    return -ERROR_NO_MEMORY;
   }
 
   offset += 4;
@@ -99,7 +99,7 @@ static int amqp_decode_array(amqp_bytes_t encoded,
       newentries = realloc(entries, allocated_entries * sizeof(amqp_field_value_t));
       if (newentries == NULL) {
 	free(entries);
-	return -ENOMEM;
+	return -ERROR_NO_MEMORY;
       }
       entries = newentries;
     }
@@ -117,7 +117,7 @@ static int amqp_decode_array(amqp_bytes_t encoded,
   if (output->entries == NULL && num_entries > 0) {
     /* NULL is legitimate if we requested a zero-length block. */
     free(entries);
-    return -ENOMEM;
+    return -ERROR_NO_MEMORY;
   }
 
   memcpy(output->entries, entries, num_entries * sizeof(amqp_field_value_t));
@@ -140,7 +140,7 @@ int amqp_decode_table(amqp_bytes_t encoded,
   int limit;
 
   if (entries == NULL) {
-    return -ENOMEM;
+    return -ERROR_NO_MEMORY;
   }
 
   offset += 4;
@@ -159,7 +159,7 @@ int amqp_decode_table(amqp_bytes_t encoded,
       newentries = realloc(entries, allocated_entries * sizeof(amqp_table_entry_t));
       if (newentries == NULL) {
 	free(entries);
-	return -ENOMEM;
+	return -ERROR_NO_MEMORY;
       }
       entries = newentries;
     }
@@ -182,7 +182,7 @@ int amqp_decode_table(amqp_bytes_t encoded,
   if (output->entries == NULL && num_entries > 0) {
     /* NULL is legitimate if we requested a zero-length block. */
     free(entries);
-    return -ENOMEM;
+    return -ERROR_NO_MEMORY;
   }
 
   memcpy(output->entries, entries, num_entries * sizeof(amqp_table_entry_t));
@@ -274,7 +274,7 @@ static int amqp_decode_field_value(amqp_bytes_t encoded,
     case AMQP_FIELD_KIND_VOID:
       break;
     default:
-      return -EINVAL;
+      return -ERROR_BAD_AMQP_DATA;
   }
 
   *offsetptr = offset;
@@ -410,7 +410,7 @@ static int amqp_encode_field_value(amqp_bytes_t encoded,
     case AMQP_FIELD_KIND_VOID:
       break;
     default:
-      return -EINVAL;
+      abort();
   }
 
   *offsetptr = offset;
diff --git a/librabbitmq/codegen.py b/librabbitmq/codegen.py
index 7aea9f4..9a82fbe 100644
--- a/librabbitmq/codegen.py
+++ b/librabbitmq/codegen.py
@@ -170,7 +170,7 @@ def genErl(spec):
         if m.arguments:
             print "      %s *m = (%s *) amqp_pool_alloc(pool, sizeof(%s));" % \
                 (m.structName(), m.structName(), m.structName())
-            print "      if (m == NULL) { return -ENOMEM; }"
+            print "      if (m == NULL) { return -ERROR_NO_MEMORY; }"
         else:
             print "      %s *m = NULL; /* no fields */" % (m.structName(),)
         bitindex = None
@@ -197,7 +197,7 @@ def genErl(spec):
         print "    case %d: {" % (c.index,)
         print "      %s *p = (%s *) amqp_pool_alloc(pool, sizeof(%s));" % \
               (c.structName(), c.structName(), c.structName())
-        print "      if (p == NULL) { return -ENOMEM; }"
+        print "      if (p == NULL) { return -ERROR_NO_MEMORY; }"
         print "      p->_flags = flags;"
         for f in c.fields:
             if spec.resolveDomain(f.domain) == 'bit':
@@ -261,8 +261,6 @@ def genErl(spec):
     print '#include <stdint.h>'
     print '#include <string.h>'
     print '#include <stdio.h>'
-    print '#include <errno.h>'
-    print '#include <arpa/inet.h> /* ntohl, htonl, ntohs, htons */'
     print
     print '#include "amqp.h"'
     print '#include "amqp_framing.h"'
@@ -317,7 +315,7 @@ int amqp_decode_method(amqp_method_number_t methodNumber,
 
   switch (methodNumber) {"""
     for m in methods: genDecodeMethodFields(m)
-    print """    default: return -ENOENT;
+    print """    default: return -ERROR_UNKNOWN_METHOD;
   }
 }"""
 
@@ -343,7 +341,7 @@ int amqp_decode_properties(uint16_t class_id,
 
   switch (class_id) {"""
     for c in spec.allClasses(): genDecodeProperties(c)
-    print """    default: return -ENOENT;
+    print """    default: return -ERROR_UNKNOWN_CLASS;
   }
 }"""
 
@@ -358,7 +356,7 @@ int amqp_encode_method(amqp_method_number_t methodNumber,
 
   switch (methodNumber) {"""
     for m in methods: genEncodeMethodFields(m)
-    print """    default: return -ENOENT;
+    print """    default: return -ERROR_UNKNOWN_METHOD;
   }
 }"""
 
@@ -390,7 +388,7 @@ int amqp_encode_properties(uint16_t class_id,
   
   switch (class_id) {"""
     for c in spec.allClasses(): genEncodeProperties(c)
-    print """    default: return -ENOENT;
+    print """    default: return -ERROR_UNKNOWN_CLASS;
   }
 }"""