[Asterisk-code-review] vector: Add options for alternate memory management (asterisk[16])
George Joseph
asteriskteam at digium.com
Fri Nov 9 11:48:05 CST 2018
George Joseph has uploaded this change for review. ( https://gerrit.asterisk.org/10612
Change subject: vector: Add options for alternate memory management
......................................................................
vector: Add options for alternate memory management
The vector implementation has been using the ast_calloc, ast_free,
and ast_malloc functions for memory management but there are
situations where the vector may be used to process error or
backtrace information where an error in the ast_ MM functions
could cause a recursive error situation.
* Added an ast_calloc_ptr function that, regardless of MALLOC_DEBUG,
always points to a real function and not a macro.
* Added an ast_std_strdup function that always calls the libc strdup
function regardless of MALLOC_DEBUG. Will be needed for a
follow-on commit.
* Added 3 virtual function pointers for calloc, free and element
cleanup to the vector structures. AST_VECTOR_INIT defaults these
to ast_calloc_ptr, ast_free_ptr and NULL respectively. This
preserves existing behavior.
* All places in vector.h that had called the ast_ functions
directly now use the virtual function pointers instead.
* A new macro AST_VECTOR_INIT_MM_FN() allows the caller to specify
the 3 new functions. The element cleanup function is optional.
* Two new macros AST_VECTOR_CLEANUP and AST_VECTOR_PTR_CLEANUP will
use the 3 virtual functions to...
1. Cleanup all elements in the vector (provided the element
cleanup function was specified in AST_VECTOR_INIT_MM_FN)
2. Use the free virtual function to free the internal "elems"
array.
3. For AST_VECTOR_PTR_CLEANUP, use the free virtual function to
free the externally allocated vector structure itself.
* cli.c was using a calloc'd vector without having called
AST_VECTOR_INIT() which used to be effectively safe but was never
technically correct. It now calls AST_VECTOR_INIT.
* optional_api.c was using a vector without having called
AST_VECTOR_INIT() on it. Needed to add init() and atexit()
routines to do the work.
Example where a function must return a vector that doesn't use the
ast_ memory management functions. Error checking omitted for
brevity.
struct ast_vector_string *get_somevector()
{
struct ast_vector_string *strings = ast_std_malloc(sizeof(*strings));
AST_VECTOR_INIT_MM_FN(strings, 0, ast_std_calloc, ast_std_free,
ast_std_free);
AST_VECTOR_APPEND(strings, ast_std_strdup(somestring1);
AST_VECTOR_APPEND(strings, ast_std_strdup(somestring2);
AST_VECTOR_APPEND(strings, ast_std_strdup(somestring3);
return strings;
}
In this case, the ast_std_calloc function will be used to allocate
the vector's elements array.
void myfunc()
{
int i;
struct ast_vector_string *strings = get_somevector();
for (i = 0; i < AST_VECTOR_SIZE(strings); i++) {
AST_LOG(LOG_ERROR, "%s\n", AST_VECTOR_GET(strings, i));
}
AST_VECTOR_PTR_CLEANUP(strings);
}
Notice that myfunc() doesn't have to worry about how either
the vector itself or the elements were allocated. It just
calls AST_VECTOR_PTR_CLEANUP() and that macro uses ast_std_free
to free the elements, the internal "elems" array, and the vector
structure itself.
Change-Id: I0d871dfed9ce215fd11aa03e7d6cfaf385b55597
---
M include/asterisk/_private.h
M include/asterisk/astmm.h
M include/asterisk/vector.h
M main/asterisk.c
M main/astmm.c
M main/cli.c
M main/optional_api.c
7 files changed, 145 insertions(+), 11 deletions(-)
git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/12/10612/1
diff --git a/include/asterisk/_private.h b/include/asterisk/_private.h
index d954768..71f5a5b 100644
--- a/include/asterisk/_private.h
+++ b/include/asterisk/_private.h
@@ -56,6 +56,7 @@
void ast_msg_shutdown(void); /*!< Provided by message.c */
int aco_init(void); /*!< Provided by config_options.c */
int dns_core_init(void); /*!< Provided by dns_core.c */
+int ast_optional_api_init(void); /*!< Provided by optional_api.c */
/*!
* \brief Initialize malloc debug phase 1.
diff --git a/include/asterisk/astmm.h b/include/asterisk/astmm.h
index e1f91dd..8b66fca 100644
--- a/include/asterisk/astmm.h
+++ b/include/asterisk/astmm.h
@@ -34,6 +34,7 @@
void *ast_std_malloc(size_t size) attribute_malloc;
void *ast_std_calloc(size_t nmemb, size_t size) attribute_malloc;
void *ast_std_realloc(void *ptr, size_t size);
+char *ast_std_strdup(const char *ptr);
void ast_std_free(void *ptr);
/*!
@@ -44,6 +45,14 @@
*/
void ast_free_ptr(void *ptr);
+/*!
+ * \brief calloc() wrapper
+ *
+ * ast_calloc_ptr should be used when a function pointer for calloc() needs to be passed
+ * as the argument to a function. Otherwise, astmm will cause seg faults.
+ */
+void *ast_calloc_ptr(size_t nmemb, size_t size);
+
void *__ast_calloc(size_t nmemb, size_t size, const char *file, int lineno, const char *func) attribute_malloc;
void *__ast_calloc_cache(size_t nmemb, size_t size, const char *file, int lineno, const char *func) attribute_malloc;
void *__ast_malloc(size_t size, const char *file, int lineno, const char *func) attribute_malloc;
diff --git a/include/asterisk/vector.h b/include/asterisk/vector.h
index d1b2973..61ddcc3 100644
--- a/include/asterisk/vector.h
+++ b/include/asterisk/vector.h
@@ -35,6 +35,9 @@
* \since 12
*/
+typedef void *(*_calloc_fn)(size_t count, size_t len);
+typedef void (*_free_fn)(void *ptr);
+
/*!
* \brief Define a vector structure
*
@@ -46,6 +49,9 @@
type *elems; \
size_t max; \
size_t current; \
+ _calloc_fn calloc_fn; \
+ _free_fn free_fn; \
+ _free_fn elem_cleanup_fn; \
}
/*! \brief Integer vector definition */
@@ -96,6 +102,9 @@
size_t max; \
size_t current; \
ast_rwlock_t lock; \
+ _calloc_fn calloc_fn; \
+ _free_fn free_fn; \
+ _free_fn elem_cleanup_fn; \
}
/*!
@@ -113,7 +122,44 @@
#define AST_VECTOR_INIT(vec, size) ({ \
size_t __size = (size); \
size_t alloc_size = __size * sizeof(*((vec)->elems)); \
- (vec)->elems = alloc_size ? ast_calloc(1, alloc_size) : NULL; \
+ (vec)->calloc_fn = (ast_calloc_ptr); \
+ (vec)->free_fn = (ast_free_ptr); \
+ (vec)->elem_cleanup_fn = NULL; \
+ (vec)->elems = alloc_size ? (vec)->calloc_fn(1, alloc_size) : NULL; \
+ (vec)->current = 0; \
+ if ((vec)->elems) { \
+ (vec)->max = __size; \
+ } else { \
+ (vec)->max = 0; \
+ } \
+ (alloc_size == 0 || (vec)->elems != NULL) ? 0 : -1; \
+})
+
+/*!
+ * \brief Initialize a vector with specified memory management functions
+ *
+ * If \a size is 0, then no space will be allocated until the vector is
+ * appended to.
+ *
+ * This macro is primarily intended for use by functions that return vectors.
+ * It enables them to hide the details of memory management and element cleanup.
+ *
+ * \param vec Vector to initialize.
+ * \param size Initial size of the vector.
+ * \param calloc_fn The zero-fill allocation function.
+ * \param free_fn The free function.
+ * \param elem_cleanup_fn An optional element cleanup function.
+ *
+ * \return 0 on success.
+ * \return Non-zero on failure.
+ */
+#define AST_VECTOR_INIT_MM_FN(vec, size, __calloc_fn, __free_fn, __elem_cleanup_fn) ({ \
+ size_t __size = (size); \
+ size_t alloc_size = __size * sizeof(*((vec)->elems)); \
+ (vec)->calloc_fn = (__calloc_fn); \
+ (vec)->free_fn = (__free_fn); \
+ (vec)->elem_cleanup_fn = (__elem_cleanup_fn); \
+ (vec)->elems = alloc_size ? (vec)->calloc_fn(1, alloc_size) : NULL; \
(vec)->current = 0; \
if ((vec)->elems) { \
(vec)->max = __size; \
@@ -172,7 +218,9 @@
* \param vec Vector to deallocate.
*/
#define AST_VECTOR_FREE(vec) do { \
- ast_free((vec)->elems); \
+ if ((vec)->free_fn) { \
+ (vec)->free_fn((vec)->elems); \
+ } \
(vec)->elems = NULL; \
(vec)->max = 0; \
(vec)->current = 0; \
@@ -187,8 +235,45 @@
* \param vec Pointer to a malloc'd vector structure.
*/
#define AST_VECTOR_PTR_FREE(vec) do { \
+ _free_fn ff = (vec)->free_fn; \
AST_VECTOR_FREE(vec); \
- ast_free(vec); \
+ if (ff) { \
+ ff(vec); \
+ } \
+} while (0)
+
+/*!
+ * \brief Completely deallocates this vector using the element cleanup function.
+ *
+ * If no element_cleanup_fn was supplied when this vector was initialized, a simple
+ * AST_VECTOR_FREE is done.
+ *
+ * \param vec Pointer to a vector structure.
+ */
+#define AST_VECTOR_CLEANUP(vec) do { \
+ size_t __idx; \
+ if ((vec)->elem_cleanup_fn) { \
+ for (__idx = 0; __idx < (vec)->current; __idx++) { \
+ (vec)->elem_cleanup_fn((vec)->elems[__idx]); \
+ } \
+ } \
+ AST_VECTOR_FREE(vec); \
+} while (0)
+
+/*!
+ * \brief Completely deallocates this malloc'd vector using the element cleanup function.
+ *
+ * If no element_cleanup_fn was supplied when this vector was initialized, a simple
+ * AST_VECTOR_PTR_FREE is done.
+ *
+ * \param vec Pointer to a malloc'd vector structure.
+ */
+#define AST_VECTOR_PTR_CLEANUP(vec) do { \
+ _free_fn ff = (vec)->free_fn; \
+ AST_VECTOR_CLEANUP(vec); \
+ if (ff) { \
+ ff(vec); \
+ } \
} while (0)
/*!
@@ -213,8 +298,11 @@
* \param vec Pointer to a malloc'd vector structure.
*/
#define AST_VECTOR_RW_PTR_FREE(vec) do { \
+ _free_fn ff = (vec)->free_fn; \
AST_VECTOR_RW_FREE(vec); \
- ast_free(vec); \
+ if (ff) { \
+ ff(vec); \
+ } \
} while(0)
/*!
@@ -225,7 +313,7 @@
do { \
if ((idx) >= (vec)->max) { \
size_t new_max = ((idx) + 1) * 2; \
- typeof((vec)->elems) new_elems = ast_calloc(1, \
+ typeof((vec)->elems) new_elems = (vec)->calloc_fn(1, \
new_max * sizeof(*new_elems)); \
if (new_elems) { \
if ((vec)->elems) { \
@@ -744,6 +832,8 @@
* elements in a new vector
*
* This macro basically provides a filtered clone.
+ * The memory management functions used for the clone will be the same as the
+ * source vector.
*
* \param vec Vector to operate on.
* \param callback A callback that takes at least 1 argument (the element)
@@ -791,12 +881,13 @@
size_t idx; \
typeof((vec)) new_vec; \
do { \
- new_vec = ast_malloc(sizeof(*new_vec)); \
+ new_vec = (vec)->calloc_fn(1, sizeof(*new_vec)); \
if (!new_vec) { \
break; \
} \
- if (AST_VECTOR_INIT(new_vec, AST_VECTOR_SIZE((vec))) != 0) { \
- ast_free(new_vec); \
+ if (AST_VECTOR_INIT_MM_FN(new_vec, AST_VECTOR_SIZE((vec)), (vec)->calloc_fn, \
+ (vec)->free_fn, (vec)->elem_cleanup_fn) != 0) { \
+ (vec)->free_fn(new_vec); \
new_vec = NULL; \
break; \
} \
diff --git a/main/asterisk.c b/main/asterisk.c
index 3354724..28c6321 100644
--- a/main/asterisk.c
+++ b/main/asterisk.c
@@ -4108,6 +4108,7 @@
ast_autoservice_init();
+ check_init(ast_optional_api_init(), "Optional API");
check_init(ast_timing_init(), "Timing");
check_init(ast_ssl_init(), "SSL");
read_pjproject_startup_options();
diff --git a/main/astmm.c b/main/astmm.c
index c845ee7..76c1a5f 100644
--- a/main/astmm.c
+++ b/main/astmm.c
@@ -1752,6 +1752,11 @@
return realloc(ptr, size);
}
+char *ast_std_strdup(const char *ptr)
+{
+ return strdup(ptr);
+}
+
void ast_std_free(void *ptr)
{
free(ptr);
@@ -1761,3 +1766,8 @@
{
ast_free(ptr);
}
+
+void *ast_calloc_ptr(size_t nmemb, size_t size)
+{
+ return ast_calloc(nmemb, size);
+}
diff --git a/main/cli.c b/main/cli.c
index cf51d0d..1331dea 100644
--- a/main/cli.c
+++ b/main/cli.c
@@ -2534,7 +2534,7 @@
char *retstr, *prevstr;
size_t max_equal;
size_t which = 0;
- struct ast_vector_string *vec = ast_calloc(1, sizeof(*vec));
+ struct ast_vector_string *vec = ast_malloc(sizeof(*vec));
/* Recursion into this function is a coding error. */
ast_assert(!ast_threadstorage_get_ptr(&completion_storage));
@@ -2543,6 +2543,8 @@
return NULL;
}
+ AST_VECTOR_INIT(vec, 0);
+
if (ast_threadstorage_set_ptr(&completion_storage, vec)) {
ast_log(LOG_ERROR, "Failed to initialize threadstorage for completion.\n");
ast_free(vec);
@@ -2607,8 +2609,7 @@
return vec;
vector_cleanup:
- AST_VECTOR_CALLBACK_VOID(vec, ast_free);
- AST_VECTOR_PTR_FREE(vec);
+ AST_VECTOR_PTR_CLEANUP(vec);
return NULL;
}
diff --git a/main/optional_api.c b/main/optional_api.c
index d63129c..75e0b22 100644
--- a/main/optional_api.c
+++ b/main/optional_api.c
@@ -18,6 +18,7 @@
#include "asterisk.h"
+#include "asterisk/_private.h"
#include "asterisk/optional_api.h"
#include "asterisk/utils.h"
#include "asterisk/vector.h"
@@ -143,6 +144,7 @@
return NULL;
}
+ AST_VECTOR_INIT(&api->users, 0);
strcpy(api->symname, symname); /* SAFE */
return api;
@@ -267,4 +269,23 @@
}
}
}
+
#endif /* defined(OPTIONAL_API) */
+
+static void optional_api_shutdown(void)
+{
+#ifdef OPTIONAL_API
+ AST_VECTOR_CLEANUP(&apis);
+#endif
+}
+
+int ast_optional_api_init(void)
+{
+#ifdef OPTIONAL_API
+ AST_VECTOR_INIT(&apis, 0);
+#endif
+ ast_register_cleanup(optional_api_shutdown);
+
+ return 0;
+}
+
--
To view, visit https://gerrit.asterisk.org/10612
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings
Gerrit-Project: asterisk
Gerrit-Branch: 16
Gerrit-MessageType: newchange
Gerrit-Change-Id: I0d871dfed9ce215fd11aa03e7d6cfaf385b55597
Gerrit-Change-Number: 10612
Gerrit-PatchSet: 1
Gerrit-Owner: George Joseph <gjoseph at digium.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20181109/f1ae25ed/attachment-0001.html>
More information about the asterisk-code-review
mailing list