[Asterisk-code-review] vector: Add options for alternate memory management (asterisk[13])
George Joseph
asteriskteam at digium.com
Fri Nov 9 11:03:37 CST 2018
George Joseph has uploaded this change for review. ( https://gerrit.asterisk.org/10611
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.
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/astmm.h
M include/asterisk/utils.h
M include/asterisk/vector.h
M main/astmm.c
M main/cli.c
M main/utils.c
6 files changed, 125 insertions(+), 12 deletions(-)
git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/11/10611/1
diff --git a/include/asterisk/astmm.h b/include/asterisk/astmm.h
index 1b00812..19e531f 100644
--- a/include/asterisk/astmm.h
+++ b/include/asterisk/astmm.h
@@ -56,7 +56,9 @@
void *ast_std_malloc(size_t size);
void *ast_std_calloc(size_t nmemb, size_t size);
+void *ast_calloc_ptr(size_t nmemb, size_t size);
void *ast_std_realloc(void *ptr, size_t size);
+char *ast_std_strdup(const char *ptr);
void ast_std_free(void *ptr);
void ast_free_ptr(void *ptr);
diff --git a/include/asterisk/utils.h b/include/asterisk/utils.h
index bcb5b24..39cc181 100644
--- a/include/asterisk/utils.h
+++ b/include/asterisk/utils.h
@@ -514,6 +514,7 @@
#define ast_std_malloc malloc
#define ast_std_calloc calloc
#define ast_std_realloc realloc
+#define ast_std_strdup strdup
#define ast_std_free free
/*!
@@ -586,6 +587,14 @@
)
/*!
+ * \brief A wrapper for calloc() for use as value assignment or function argument
+ *
+ * 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.
+ */
+#define ast_calloc_ptr calloc
+
+/*!
* \brief A wrapper for calloc() for use in cache pools
*
* ast_calloc_cache() is a wrapper for calloc() that will generate an Asterisk log
diff --git a/include/asterisk/vector.h b/include/asterisk/vector.h
index 2ee5e29..29f78cf 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 */
@@ -66,6 +72,9 @@
size_t max; \
size_t current; \
ast_rwlock_t lock; \
+ _calloc_fn calloc_fn; \
+ _free_fn free_fn; \
+ _free_fn elem_cleanup_fn; \
}
/*!
@@ -83,7 +92,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; \
@@ -142,7 +188,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; \
@@ -157,8 +205,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)
/*!
@@ -183,8 +268,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)
/*!
@@ -195,7 +283,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) { \
@@ -714,6 +802,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)
@@ -761,12 +851,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/astmm.c b/main/astmm.c
index 2134a1f..d8c98e2 100644
--- a/main/astmm.c
+++ b/main/astmm.c
@@ -170,11 +170,21 @@
return calloc(nmemb, size);
}
+void *ast_calloc_ptr(size_t nmemb, size_t size)
+{
+ return ast_calloc(nmemb, size);
+}
+
void *ast_std_realloc(void *ptr, size_t size)
{
return realloc(ptr, size);
}
+char *ast_std_strdup(const char *ptr)
+{
+ return strdup(ptr);
+}
+
void ast_std_free(void *ptr)
{
free(ptr);
diff --git a/main/cli.c b/main/cli.c
index dc75acb..9744057 100644
--- a/main/cli.c
+++ b/main/cli.c
@@ -2562,7 +2562,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));
@@ -2571,6 +2571,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);
@@ -2635,8 +2637,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/utils.c b/main/utils.c
index 9441187..f2e5d52 100644
--- a/main/utils.c
+++ b/main/utils.c
@@ -976,9 +976,9 @@
/* store frame count locally to avoid the memory corruption that
* sometimes happens on virtualized CentOS 6.x systems */
num_frames = bt->num_frames;
+
if ((symbols = ast_bt_get_symbols(bt->addresses, num_frames))) {
int frame_iterator;
-
for (frame_iterator = 0; frame_iterator < num_frames; ++frame_iterator) {
ast_str_append(str, 0, "\t%s\n", symbols[frame_iterator]);
}
--
To view, visit https://gerrit.asterisk.org/10611
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-MessageType: newchange
Gerrit-Change-Id: I0d871dfed9ce215fd11aa03e7d6cfaf385b55597
Gerrit-Change-Number: 10611
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/e54f8d7e/attachment-0001.html>
More information about the asterisk-code-review
mailing list