Eliminate malloc in spdylay_map

We use intrusive style pattern in spdylay_stream, which now has
spdylay_map_entry has its first member.

lib/spdylay_map.c

@@ -30,25 +30,28 @@ void spdylay_map_init(spdylay_map *map)
   map->size = 0;
 }
 
-static void spdylay_map_entry_free(spdylay_map_entry *entry)
+static void spdylay_map_entry_free_recur(spdylay_map_entry *entry,
+                                         int (*func)(spdylay_map_entry *entry,
+                                                     void *ptr),
+                                         void *ptr)
 {
   if(entry != NULL) {
-    free(entry);
+    spdylay_map_entry_free_recur(entry->left, func, ptr);
-  }
+    spdylay_map_entry_free_recur(entry->right, func, ptr);
-}
+    func(entry, ptr);
-
-static void spdylay_map_entry_free_recur(spdylay_map_entry *entry)
-{
-  if(entry != NULL) {
-    spdylay_map_entry_free_recur(entry->left);
-    spdylay_map_entry_free_recur(entry->right);
-    free(entry);
   }
 }
 
 void spdylay_map_free(spdylay_map *map)
 {
-  spdylay_map_entry_free_recur(map->root);
+  map->root = NULL;
+}
+
+void spdylay_map_each_free(spdylay_map *map,
+                           int (*func)(spdylay_map_entry *entry, void *ptr),
+                           void *ptr)
+{
+  spdylay_map_entry_free_recur(map->root, func, ptr);
   map->root = NULL;
 }
 
@@ -68,17 +71,11 @@ static uint32_t hash32shift(uint32_t key)
   return key;
 }
 
-static spdylay_map_entry* spdylay_map_entry_new(key_type key, void *val)
+void spdylay_map_entry_init(spdylay_map_entry *entry, key_type key)
 {
-  spdylay_map_entry *entry =
-    (spdylay_map_entry*)malloc(sizeof(spdylay_map_entry));
-  if(entry != NULL) {
   entry->key = key;
-    entry->val = val;
   entry->left = entry->right = NULL;
   entry->priority = hash32shift(key);
-  }
-  return entry;
 }
 
 static spdylay_map_entry* rotate_left(spdylay_map_entry *entry)
@@ -98,21 +95,17 @@ static spdylay_map_entry* rotate_right(spdylay_map_entry* entry)
 }
 
 static spdylay_map_entry* insert_recur(spdylay_map_entry *entry,
-                                       key_type key, void *val,
+                                       spdylay_map_entry *new_entry,
                                        int *error)
 {
   if(entry == NULL) {
-    entry = spdylay_map_entry_new(key, val);
+    entry = new_entry;
-    if(entry == NULL) {
+  } else if(new_entry->key == entry->key) {
-      *error = SPDYLAY_ERR_NOMEM;
-      return NULL;
-    }
-  } else if(key == entry->key) {
     *error = SPDYLAY_ERR_INVALID_ARGUMENT;
-  } else if(key < entry->key) {
+  } else if(new_entry->key < entry->key) {
-    entry->left = insert_recur(entry->left, key, val, error);
+    entry->left = insert_recur(entry->left, new_entry, error);
   } else {
-    entry->right = insert_recur(entry->right, key, val, error);
+    entry->right = insert_recur(entry->right, new_entry, error);
   }
   if(entry->left != NULL && entry->priority > entry->left->priority) {
     entry = rotate_right(entry);
@@ -122,17 +115,17 @@ static spdylay_map_entry* insert_recur(spdylay_map_entry *entry,
   return entry;
 }
 
-int spdylay_map_insert(spdylay_map *map, key_type key, void *val)
+int spdylay_map_insert(spdylay_map *map, spdylay_map_entry *new_entry)
 {
   int error = 0;
-  map->root = insert_recur(map->root, key, val, &error);
+  map->root = insert_recur(map->root, new_entry, &error);
   if(!error) {
     ++map->size;
   }
   return error;
 }
 
-void* spdylay_map_find(spdylay_map *map, key_type key)
+spdylay_map_entry* spdylay_map_find(spdylay_map *map, key_type key)
 {
   spdylay_map_entry *entry = map->root;
   while(entry != NULL) {
@@ -141,57 +134,57 @@ void* spdylay_map_find(spdylay_map *map, key_type key)
     } else if(key > entry->key) {
       entry = entry->right;
     } else {
-      return entry->val;
+      return entry;
     }
   }
   return NULL;
 }
 
-static spdylay_map_entry* erase_rotate_recur(spdylay_map_entry *entry)
+static spdylay_map_entry* remove_rotate_recur(spdylay_map_entry *entry)
 {
   if(entry->left == NULL) {
     spdylay_map_entry *right = entry->right;
-    spdylay_map_entry_free(entry);
     return right;
   } else if(entry->right == NULL) {
     spdylay_map_entry *left = entry->left;
-    spdylay_map_entry_free(entry);
     return left;
   } else if(entry->left->priority < entry->right->priority) {
     entry = rotate_right(entry);
-    entry->right = erase_rotate_recur(entry->right);
+    entry->right = remove_rotate_recur(entry->right);
     return entry;
   } else {
     entry = rotate_left(entry);
-    entry->left = erase_rotate_recur(entry->left);
+    entry->left = remove_rotate_recur(entry->left);
     return entry;
   }
 }
 
-static spdylay_map_entry* erase_recur(spdylay_map_entry *entry, key_type key,
+static spdylay_map_entry* remove_recur(spdylay_map_entry *entry, key_type key,
                                       int *error)
 {
   if(entry == NULL) {
     *error = SPDYLAY_ERR_INVALID_ARGUMENT;
   } else if(key < entry->key) {
-    entry->left = erase_recur(entry->left, key, error);
+    entry->left = remove_recur(entry->left, key, error);
   } else if(key > entry->key) {
-    entry->right = erase_recur(entry->right, key, error);
+    entry->right = remove_recur(entry->right, key, error);
   } else {
-    entry = erase_rotate_recur(entry);
+    entry = remove_rotate_recur(entry);
   }
   return entry;
 }
 
-void spdylay_map_erase(spdylay_map *map, key_type key)
+int spdylay_map_remove(spdylay_map *map, key_type key)
 {
   if(map->root != NULL) {
     int error = 0;
-    map->root = erase_recur(map->root, key, &error);
+    map->root = remove_recur(map->root, key, &error);
     if(!error) {
       --map->size;
     }
+    return error;
   }
+  return SPDYLAY_ERR_INVALID_ARGUMENT;
 }
 
 size_t spdylay_map_size(spdylay_map *map)
@@ -200,13 +193,13 @@ size_t spdylay_map_size(spdylay_map *map)
 }
 
 static int for_each(spdylay_map_entry *entry,
-                    int (*func)(key_type key, void *val, void *ptr),
+                    int (*func)(spdylay_map_entry *entry, void *ptr),
                     void *ptr)
 {
   if(entry) {
     int rv;
     if((rv = for_each(entry->left, func, ptr)) != 0 ||
-       (rv = func(entry->key, entry->val, ptr)) != 0 ||
+       (rv = func(entry, ptr)) != 0 ||
        (rv = for_each(entry->right, func, ptr)) != 0) {
       return rv;
     }
@@ -215,7 +208,7 @@ static int for_each(spdylay_map_entry *entry,
 }
 
 int spdylay_map_each(spdylay_map *map,
-                     int (*func)(key_type key, void *val, void *ptr),
+                     int (*func)(spdylay_map_entry *entry, void *ptr),
                      void *ptr)
 {
   return for_each(map->root, func, ptr);

lib/spdylay_map.h

@@ -39,7 +39,6 @@ typedef uint32_t pri_type;
 
 typedef struct spdylay_map_entry {
   key_type key;
-  void *val;
   struct spdylay_map_entry *left, *right;
   pri_type priority;
 } spdylay_map_entry;
@@ -55,43 +54,56 @@ typedef struct {
 void spdylay_map_init(spdylay_map *map);
 
 /*
- * Deallocates any resources allocated for |map|. The stored items are
+ * Deallocates any resources allocated for |map|. The stored entries
- * not freed by this function. Use spdylay_map_each() to free each
+ * are not freed by this function. Use spdylay_map_each_free() to free
- * item.
+ * each entries.
  */
 void spdylay_map_free(spdylay_map *map);
 
 /*
- * Inserts the new item |val| with the key |key| to the map |map|.
+ * Deallocates each entries using |func| function and any resources
+ * allocated for |map|. The |func| function is responsible for freeing
+ * given the |entry| object. The |ptr| will be passed to the |func| as
+ * send argument. The return value of the |func| will be ignored.
+ */
+void spdylay_map_each_free(spdylay_map *map,
+                           int (*func)(spdylay_map_entry *entry, void *ptr),
+                           void *ptr);
+
+/*
+ * Initializes the |entry| with the |key|. All entries to be inserted
+ * to the map must be initialized with this function.
+ */
+void spdylay_map_entry_init(spdylay_map_entry *entry, key_type key);
+
+/*
+ * Inserts the new |entry| with the key |entry->key| to the map |map|.
  *
  * This function returns 0 if it succeeds, or one of the following
  * negative error code:
  *
  * SPDYLAY_ERR_INVALID_ARGUMENT
  *     The item associated by |key| already exists.
- *
- * SPDYLAY_ERR_NOMEM
- *     Out of memory.
  */
-int spdylay_map_insert(spdylay_map *map, key_type key, void *val);
+int spdylay_map_insert(spdylay_map *map, spdylay_map_entry *entry);
 
 /*
- * Returns the item associated by the key |key|.  If there is no such
+ * Returns the entry associated by the key |key|.  If there is no such
- * item, this function returns NULL.
+ * entry, this function returns NULL.
  */
-void* spdylay_map_find(spdylay_map *map, key_type key);
+spdylay_map_entry* spdylay_map_find(spdylay_map *map, key_type key);
 
 /*
- * Erases the item associated by the key |key|.  The erased item is
+ * Removes the entry associated by the key |key| from the |map|.  The
- * not freed by this function.
+ * removed entry is not freed by this function.
  *
  * This function returns 0 if it succeeds, or one of the following
  * negative error codes:
  *
  * SPDYLAY_ERR_INVALID_ARGUMENT
- *     The item associated by |key| does not exist.
+ *     The entry associated by |key| does not exist.
  */
-void spdylay_map_erase(spdylay_map *map, key_type key);
+int spdylay_map_remove(spdylay_map *map, key_type key);
 
 /*
  * Returns the number of items stored in the map |map|.
@@ -99,19 +111,21 @@ void spdylay_map_erase(spdylay_map *map, key_type key);
 size_t spdylay_map_size(spdylay_map *map);
 
 /*
- * Applies the function |func| to each key/item pair in the map |map|
+ * Applies the function |func| to each entry in the |map| with the
- * with the optional user supplied pointer |ptr|.  This function is
+ * optional user supplied pointer |ptr|.
- * useful to free item in the map.
  *
  * If the |func| returns 0, this function calls the |func| with the
- * next key and value pair. If the |func| returns nonzero, it will not
+ * next entry. If the |func| returns nonzero, it will not call the
- * call the |func| for further key and value pair and return the
+ * |func| for further entries and return the return value of the
- * return value of the |func| immediately.  Thus, this function
+ * |func| immediately.  Thus, this function returns 0 if all the
- * returns 0 if all the invocations of the |func| return 0, or nonzero
+ * invocations of the |func| return 0, or nonzero value which the last
- * value which the last invocation of |func| returns.
+ * invocation of |func| returns.
+ *
+ * Don't use this function to free each entry. Use
+ * spdylay_map_each_free() instead.
  */
 int spdylay_map_each(spdylay_map *map,
-                     int (*func)(key_type key, void *val, void *ptr),
+                     int (*func)(spdylay_map_entry *entry, void *ptr),
                      void *ptr);
 
 #endif /* SPDYLAY_MAP_H */

lib/spdylay_session.c

@@ -252,7 +252,7 @@ static int spdylay_session_new(spdylay_session **session_ptr,
  fail_ob_ss_pq:
   spdylay_pq_free(&(*session_ptr)->ob_pq);
  fail_ob_pq:
-  spdylay_map_free(&(*session_ptr)->streams);
+  /* No need to free (*session_ptr)->streams) here. */
   spdylay_zlib_inflate_free(&(*session_ptr)->hd_inflater);
  fail_hd_inflater:
   spdylay_zlib_deflate_free(&(*session_ptr)->hd_deflater);
@@ -297,10 +297,10 @@ int spdylay_session_server_new(spdylay_session **session_ptr,
   return r;
 }
 
-static int spdylay_free_streams(key_type key, void *val, void *ptr)
+static int spdylay_free_streams(spdylay_map_entry *entry, void *ptr)
 {
-  spdylay_stream_free((spdylay_stream*)val);
+  spdylay_stream_free((spdylay_stream*)entry);
-  free(val);
+  free(entry);
   return 0;
 }
 
@@ -329,8 +329,7 @@ void spdylay_session_del(spdylay_session *session)
   if(session == NULL) {
     return;
   }
-  spdylay_map_each(&session->streams, spdylay_free_streams, NULL);
+  spdylay_map_each_free(&session->streams, spdylay_free_streams, NULL);
-  spdylay_map_free(&session->streams);
   spdylay_session_ob_pq_free(&session->ob_pq);
   spdylay_session_ob_pq_free(&session->ob_ss_pq);
   spdylay_zlib_deflate_free(&session->hd_deflater);
@@ -479,7 +478,7 @@ spdylay_stream* spdylay_session_open_stream(spdylay_session *session,
                       session->remote_settings
                       [SPDYLAY_SETTINGS_INITIAL_WINDOW_SIZE],
                       stream_user_data);
-  r = spdylay_map_insert(&session->streams, stream_id, stream);
+  r = spdylay_map_insert(&session->streams, &stream->map_entry);
   if(r != 0) {
     free(stream);
     stream = NULL;
@@ -508,7 +507,7 @@ int spdylay_session_close_stream(spdylay_session *session, int32_t stream_id,
     } else {
       --session->num_incoming_streams;
     }
-    spdylay_map_erase(&session->streams, stream_id);
+    spdylay_map_remove(&session->streams, stream_id);
     spdylay_stream_free(stream);
     free(stream);
     return 0;
@@ -1828,13 +1827,13 @@ int spdylay_session_on_rst_stream_received(spdylay_session *session,
   return 0;
 }
 
-static int spdylay_update_initial_window_size_func(key_type key, void *value,
+static int spdylay_update_initial_window_size_func(spdylay_map_entry *entry,
                                                    void *ptr)
 {
   spdylay_update_window_size_arg *arg;
   spdylay_stream *stream;
   arg = (spdylay_update_window_size_arg*)ptr;
-  stream = (spdylay_stream*)value;
+  stream = (spdylay_stream*)entry;
   spdylay_stream_update_initial_window_size(stream,
                                             arg->new_window_size,
                                             arg->old_window_size);

lib/spdylay_stream.c

@@ -32,6 +32,7 @@ void spdylay_stream_init(spdylay_stream *stream, int32_t stream_id,
                          int32_t initial_window_size,
                          void *stream_user_data)
 {
+  spdylay_map_entry_init(&stream->map_entry, stream_id);
   stream->stream_id = stream_id;
   stream->flags = flags;
   stream->pri = pri;

lib/spdylay_stream.h

@@ -31,6 +31,7 @@
 
 #include <spdylay/spdylay.h>
 #include "spdylay_outbound_item.h"
+#include "spdylay_map.h"
 
 /*
  * If local peer is stream initiator:
@@ -76,6 +77,9 @@ typedef enum {
 } spdylay_deferred_flag;
 
 typedef struct {
+  /* Intrusive Map */
+  spdylay_map_entry map_entry;
+  /* stream ID */
   int32_t stream_id;
   spdylay_stream_state state;
   /* Use same value in SYN_STREAM frame */

tests/main.c

@@ -68,6 +68,7 @@ int main(int argc, char* argv[])
    /* add the tests to the suite */
    if(!CU_add_test(pSuite, "pq", test_spdylay_pq) ||
       !CU_add_test(pSuite, "map", test_spdylay_map) ||
+      !CU_add_test(pSuite, "map_each_free", test_spdylay_map_each_free) ||
       !CU_add_test(pSuite, "queue", test_spdylay_queue) ||
       !CU_add_test(pSuite, "buffer", test_spdylay_buffer) ||
       !CU_add_test(pSuite, "buffer_reader", test_spdylay_buffer_reader) ||

tests/spdylay_map_test.c

@@ -28,39 +28,94 @@
 
 #include "spdylay_map.h"
 
+typedef struct strentry {
+  spdylay_map_entry map_entry;
+  const char *str;
+} strentry;
+
+static void strentry_init(strentry *entry, key_type key, const char *str)
+{
+  spdylay_map_entry_init(&entry->map_entry, key);
+  entry->str = str;
+}
+
 void test_spdylay_map(void)
 {
+  strentry foo, FOO, bar, baz, shrubbery;
   spdylay_map map;
   spdylay_map_init(&map);
-  CU_ASSERT(0 == spdylay_map_insert(&map, 1, (void*)"foo"));
-  CU_ASSERT(strcmp("foo", spdylay_map_find(&map, 1)) == 0);
-  CU_ASSERT(1 == spdylay_map_size(&map));
-  CU_ASSERT(SPDYLAY_ERR_INVALID_ARGUMENT == spdylay_map_insert(&map, 1,
-                                                               (void*)"FOO"));
-  CU_ASSERT(1 == spdylay_map_size(&map));
-  CU_ASSERT(strcmp("foo", spdylay_map_find(&map, 1)) == 0);
-  CU_ASSERT(0 == spdylay_map_insert(&map, 2, (void*)"bar"));
-  CU_ASSERT(2 == spdylay_map_size(&map));
-  CU_ASSERT(0 == spdylay_map_insert(&map, 3, (void*)"baz"));
-  CU_ASSERT(3 == spdylay_map_size(&map));
-  CU_ASSERT(0 == spdylay_map_insert(&map, 4, (void*)"shrubbery"));
-  CU_ASSERT(4 == spdylay_map_size(&map));
-  CU_ASSERT(strcmp("baz", spdylay_map_find(&map, 3)) == 0);
 
-  spdylay_map_erase(&map, 3);
+  strentry_init(&foo, 1, "foo");
+  strentry_init(&FOO, 1, "FOO");
+  strentry_init(&bar, 2, "bar");
+  strentry_init(&baz, 3, "baz");
+  strentry_init(&shrubbery, 4, "shrubbery");
+
+  CU_ASSERT(0 == spdylay_map_insert(&map, &foo.map_entry));
+  CU_ASSERT(strcmp("foo", ((strentry*)spdylay_map_find(&map, 1))->str) == 0);
+  CU_ASSERT(1 == spdylay_map_size(&map));
+
+  CU_ASSERT(SPDYLAY_ERR_INVALID_ARGUMENT ==
+            spdylay_map_insert(&map, &FOO.map_entry));
+
+  CU_ASSERT(1 == spdylay_map_size(&map));
+  CU_ASSERT(strcmp("foo", ((strentry*)spdylay_map_find(&map, 1))->str) == 0);
+
+  CU_ASSERT(0 == spdylay_map_insert(&map, &bar.map_entry));
+  CU_ASSERT(2 == spdylay_map_size(&map));
+
+  CU_ASSERT(0 == spdylay_map_insert(&map, &baz.map_entry));
+  CU_ASSERT(3 == spdylay_map_size(&map));
+
+  CU_ASSERT(0 == spdylay_map_insert(&map, &shrubbery.map_entry));
+  CU_ASSERT(4 == spdylay_map_size(&map));
+
+  CU_ASSERT(strcmp("baz", ((strentry*)spdylay_map_find(&map, 3))->str) == 0);
+
+  spdylay_map_remove(&map, 3);
   CU_ASSERT(3 == spdylay_map_size(&map));
   CU_ASSERT(NULL == spdylay_map_find(&map, 3));
-  spdylay_map_erase(&map, 1);
+
+  spdylay_map_remove(&map, 1);
   CU_ASSERT(2 == spdylay_map_size(&map));
   CU_ASSERT(NULL == spdylay_map_find(&map, 1));
 
   /* Erasing non-existent entry */
-  spdylay_map_erase(&map, 1);
+  spdylay_map_remove(&map, 1);
   CU_ASSERT(2 == spdylay_map_size(&map));
   CU_ASSERT(NULL == spdylay_map_find(&map, 1));
 
-  CU_ASSERT(strcmp("bar", spdylay_map_find(&map, 2)) == 0);
+  CU_ASSERT(strcmp("bar", ((strentry*)spdylay_map_find(&map, 2))->str) == 0);
-  CU_ASSERT(strcmp("shrubbery", spdylay_map_find(&map, 4)) == 0);
+  CU_ASSERT(strcmp("shrubbery",
+                   ((strentry*)spdylay_map_find(&map, 4))->str) == 0);
 
   spdylay_map_free(&map);
 }
+
+static int entry_free(spdylay_map_entry *entry, void *ptr)
+{
+  free(entry);
+  return 0;
+}
+
+void test_spdylay_map_each_free(void)
+{
+  strentry *foo = malloc(sizeof(strentry)),
+    *bar = malloc(sizeof(strentry)),
+    *baz = malloc(sizeof(strentry)),
+    *shrubbery = malloc(sizeof(strentry));
+  spdylay_map map;
+  spdylay_map_init(&map);
+
+  strentry_init(foo, 1, "foo");
+  strentry_init(bar, 2, "bar");
+  strentry_init(baz, 3, "baz");
+  strentry_init(shrubbery, 4, "shrubbery");
+
+  spdylay_map_insert(&map, &foo->map_entry);
+  spdylay_map_insert(&map, &bar->map_entry);
+  spdylay_map_insert(&map, &baz->map_entry);
+  spdylay_map_insert(&map, &shrubbery->map_entry);
+
+  spdylay_map_each_free(&map, entry_free, NULL);
+}

tests/spdylay_map_test.h

@@ -26,5 +26,6 @@
 #define SPDYLAY_MAP_TEST_H
 
 void test_spdylay_map(void);
+void test_spdylay_map_each_free(void);
 
 #endif /* SPDYLAY_MAP_TEST_H */