]> git.ipfire.org Git - thirdparty/asterisk.git/commitdiff
app_queue: Support persisting and loading of long member lists.
authorSean Bright <sean@malleable.com>
Mon, 1 Oct 2012 16:45:53 +0000 (16:45 +0000)
committerSean Bright <sean@malleable.com>
Mon, 1 Oct 2012 16:45:53 +0000 (16:45 +0000)
Greenlight in #asterisk brought up that he was receiving an error message "Could
not create persistent member string, out of space" when running app_queue in
Asterisk 10.  dump_queue_members() made an assumption that 8K would be enough to
store the generated string, but with queues that have large member lists this is
not always the case.  This patch removes the limitation and uses ast_str instead
of a fixed sized buffer.

The complicating factor comes from the fact that ast_db_get requires a buffer
and buffer size argument, which doesn't let us pull back more than what we pass
in, so I introduced a new ast_db_get_allocated() which returns an ast_strdup()'d
copy of the value from astdb.

As an aside, I did some testing on the maximum size of data that we can store in
the BDB library we distribute and was able to store a 10MB string and retrieve
it with no problems, so I feel this is a safe patch.

Review: https://reviewboard.asterisk.org/r/2136/

git-svn-id: https://origsvn.digium.com/svn/asterisk/branches/1.8@374108 65c4cc65-6c06-0410-ace0-fbb531ad65f3

apps/app_queue.c
include/asterisk/astdb.h
main/db.c
tests/test_db.c

index 167f272c6b3002af841342e913b6eaad6a1a6070..1d6c52f6082ac3034486faf26c9ddc12b5057dab 100644 (file)
@@ -912,8 +912,6 @@ static char *app_ql = "QueueLog" ;
 
 /*! \brief Persistent Members astdb family */
 static const char * const pm_family = "Queue/PersistentMembers";
-/* The maximum length of each persistent member queue database entry */
-#define PM_MAX_LEN 8192
 
 /*! \brief queues.conf [general] option */
 static int queue_persistent_members = 0;
@@ -5222,15 +5220,18 @@ static struct member *interface_exists(struct call_queue *q, const char *interfa
 static void dump_queue_members(struct call_queue *pm_queue)
 {
        struct member *cur_member;
-       char value[PM_MAX_LEN];
-       int value_len = 0;
-       int res;
+       struct ast_str *value;
        struct ao2_iterator mem_iter;
 
-       memset(value, 0, sizeof(value));
+       if (!pm_queue) {
+               return;
+       }
 
-       if (!pm_queue)
+       /* 4K is a reasonable default for most applications, but we grow to
+        * accommodate more if necessary. */
+       if (!(value = ast_str_create(4096))) {
                return;
+       }
 
        mem_iter = ao2_iterator_init(pm_queue->members, 0);
        while ((cur_member = ao2_iterator_next(&mem_iter))) {
@@ -5239,25 +5240,27 @@ static void dump_queue_members(struct call_queue *pm_queue)
                        continue;
                }
 
-               res = snprintf(value + value_len, sizeof(value) - value_len, "%s%s;%d;%d;%s;%s",
-                       value_len ? "|" : "", cur_member->interface, cur_member->penalty, cur_member->paused, cur_member->membername, cur_member->state_interface);
+               ast_str_append(&value, 0, "%s%s;%d;%d;%s;%s",
+                       ast_str_strlen(value) ? "|" : "",
+                       cur_member->interface,
+                       cur_member->penalty,
+                       cur_member->paused,
+                       cur_member->membername,
+                       cur_member->state_interface);
 
                ao2_ref(cur_member, -1);
-
-               if (res != strlen(value + value_len)) {
-                       ast_log(LOG_WARNING, "Could not create persistent member string, out of space\n");
-                       break;
-               }
-               value_len += res;
        }
        ao2_iterator_destroy(&mem_iter);
-       
-       if (value_len && !cur_member) {
-               if (ast_db_put(pm_family, pm_queue->name, value))
+
+       if (ast_str_strlen(value) && !cur_member) {
+               if (ast_db_put(pm_family, pm_queue->name, ast_str_buffer(value)))
                        ast_log(LOG_WARNING, "failed to create persistent dynamic entry!\n");
-       } else
+       } else {
                /* Delete the entry if the queue is empty or there is an error */
                ast_db_del(pm_family, pm_queue->name);
+       }
+
+       ast_free(value);
 }
 
 /*! \brief Remove member from queue 
@@ -5538,7 +5541,7 @@ static void reload_queue_members(void)
        struct ast_db_entry *db_tree;
        struct ast_db_entry *entry;
        struct call_queue *cur_queue;
-       char queue_data[PM_MAX_LEN];
+       char *queue_data;
 
        /* Each key in 'pm_family' is the name of a queue */
        db_tree = ast_db_gettree(pm_family, NULL);
@@ -5564,7 +5567,7 @@ static void reload_queue_members(void)
                        continue;
                } 
 
-               if (ast_db_get(pm_family, queue_name, queue_data, PM_MAX_LEN)) {
+               if (ast_db_get_allocated(pm_family, queue_name, &queue_data)) {
                        queue_t_unref(cur_queue, "Expire reload reference");
                        continue;
                }
@@ -5608,6 +5611,7 @@ static void reload_queue_members(void)
                        }
                }
                queue_t_unref(cur_queue, "Expire reload reference");
+               ast_free(queue_data);
        }
 
        if (db_tree) {
index cfbebbc30140acb230b8d22f5058af113e595189..e27a6515da488a9554db28443dc50eefd809967e 100644 (file)
@@ -36,6 +36,17 @@ struct ast_db_entry {
 /*!\brief Get key value specified by family/key */
 int ast_db_get(const char *family, const char *key, char *out, int outlen);
 
+/*!\brief Get key value specified by family/key as a heap allocated string.
+ *
+ * Given a \a family and \a key, sets \a out to a pointer to a heap
+ * allocated string.  In the event of an error, \a out will be set to
+ * NULL.  The string must be freed by calling ast_free().
+ *
+ * \retval -1 An error occurred
+ * \retval 0 Success
+ */
+int ast_db_get_allocated(const char *family, const char *key, char **out);
+
 /*!\brief Store value addressed by family/key */
 int ast_db_put(const char *family, const char *key, const char *value);
 
index cf455b69c648dccae6bc7fa8bd3619a13f2b8523..74cf070c12fd218ee561d3fff834d7cdad976c47 100644 (file)
--- a/main/db.c
+++ b/main/db.c
@@ -271,7 +271,21 @@ int ast_db_put(const char *family, const char *keys, const char *value)
        return res;
 }
 
-int ast_db_get(const char *family, const char *keys, char *value, int valuelen)
+/*!
+ * \internal
+ * \brief Get key value specified by family/key.
+ *
+ * Gets the value associated with the specified \a family and \a keys, and
+ * stores it, either into the fixed sized buffer specified by \a buffer
+ * and \a bufferlen, or as a heap allocated string if \a bufferlen is -1.
+ *
+ * \note If \a bufferlen is -1, \a buffer points to heap allocated memory
+ *       and must be freed by calling ast_free().
+ *
+ * \retval -1 An error occurred
+ * \retval 0 Success
+ */
+static int db_get_common(const char *family, const char *keys, char **buffer, int bufferlen)
 {
        char fullkey[MAX_DB_FIELD] = "";
        DBT key, data;
@@ -286,7 +300,6 @@ int ast_db_get(const char *family, const char *keys, char *value, int valuelen)
        fullkeylen = snprintf(fullkey, sizeof(fullkey), "/%s/%s", family, keys);
        memset(&key, 0, sizeof(key));
        memset(&data, 0, sizeof(data));
-       memset(value, 0, valuelen);
        key.data = fullkey;
        key.size = fullkeylen + 1;
 
@@ -296,13 +309,16 @@ int ast_db_get(const char *family, const char *keys, char *value, int valuelen)
        if (res) {
                ast_debug(1, "Unable to find key '%s' in family '%s'\n", keys, family);
        } else {
-#if 0
-               printf("Got value of size %d\n", data.size);
-#endif
                if (data.size) {
                        ((char *)data.data)[data.size - 1] = '\0';
-                       /* Make sure that we don't write too much to the dst pointer or we don't read too much from the source pointer */
-                       ast_copy_string(value, data.data, (valuelen > data.size) ? data.size : valuelen);
+
+                       if (bufferlen == -1) {
+                               *buffer = ast_strdup(data.data);
+                       } else {
+                               /* Make sure that we don't write too much to the dst pointer or we don't
+                                * read too much from the source pointer */
+                               ast_copy_string(*buffer, data.data, bufferlen > data.size ? data.size : bufferlen);
+                       }
                } else {
                        ast_log(LOG_NOTICE, "Strange, empty value for /%s/%s\n", family, keys);
                }
@@ -315,6 +331,23 @@ int ast_db_get(const char *family, const char *keys, char *value, int valuelen)
        return res;
 }
 
+int ast_db_get(const char *family, const char *keys, char *value, int valuelen)
+{
+       ast_assert(value != NULL);
+
+       /* Make sure we initialize */
+       value[0] = 0;
+
+       return db_get_common(family, keys, &value, valuelen);
+}
+
+int ast_db_get_allocated(const char *family, const char *keys, char **out)
+{
+       *out = NULL;
+
+       return db_get_common(family, keys, out, -1);
+}
+
 int ast_db_del(const char *family, const char *keys)
 {
        char fullkey[MAX_DB_FIELD];
index 8dfc8d482655023f3c09890c5c28afd5f3c437e3..c23add62d866a15fc2898dc5420fb654833e216b 100644 (file)
@@ -206,10 +206,68 @@ AST_TEST_DEFINE(gettree_deltree)
        return res;
 }
 
+AST_TEST_DEFINE(put_get_long)
+{
+       int res = AST_TEST_PASS;
+       struct ast_str *s;
+       int i, j;
+
+#define STR_FILL_32 "abcdefghijklmnopqrstuvwxyz123456"
+
+       switch (cmd) {
+       case TEST_INIT:
+               info->name = "put_get_long";
+               info->category = "/main/astdb/";
+               info->summary = "ast_db_(put|get_allocated) unit test";
+               info->description =
+                       "Ensures that the ast_db_put and ast_db_get_allocated functions work";
+               return AST_TEST_NOT_RUN;
+       case TEST_EXECUTE:
+               break;
+       }
+
+       if (!(s = ast_str_create(4096))) {
+               return AST_TEST_FAIL;
+       }
+
+       for (i = 1024; i <= 1024 * 1024 * 8; i *= 2) {
+               char *out = NULL;
+
+               ast_str_reset(s);
+
+               for (j = 0; j < i; j += sizeof(STR_FILL_32) - 1) {
+                       ast_str_append(&s, 0, "%s", STR_FILL_32);
+               }
+
+               if (ast_db_put("astdbtest", "long", ast_str_buffer(s))) {
+                       ast_test_status_update(test, "Failed to put value of %zu bytes\n", ast_str_strlen(s));
+                       res = AST_TEST_FAIL;
+               } else if (ast_db_get_allocated("astdbtest", "long", &out)) {
+                       ast_test_status_update(test, "Failed to get value of %zu bytes\n", ast_str_strlen(s));
+                       res = AST_TEST_FAIL;
+               } else if (strcmp(ast_str_buffer(s), out)) {
+                       ast_test_status_update(test, "Failed to match value of %zu bytes\n", ast_str_strlen(s));
+                       res = AST_TEST_FAIL;
+               } else if (ast_db_del("astdbtest", "long")) {
+                       ast_test_status_update(test, "Failed to delete astdbtest/long\n");
+                       res = AST_TEST_FAIL;
+               }
+
+               if (out) {
+                       ast_free(out);
+               }
+       }
+
+       ast_free(s);
+
+       return res;
+}
+
 static int unload_module(void)
 {
        AST_TEST_UNREGISTER(put_get_del);
        AST_TEST_UNREGISTER(gettree_deltree);
+       AST_TEST_UNREGISTER(put_get_long);
        return 0;
 }
 
@@ -217,6 +275,7 @@ static int load_module(void)
 {
        AST_TEST_REGISTER(put_get_del);
        AST_TEST_REGISTER(gettree_deltree);
+       AST_TEST_REGISTER(put_get_long);
        return AST_MODULE_LOAD_SUCCESS;
 }