[Asterisk-code-review] optional api: Refactor to use vector's and standard allocators. (asterisk[master])
Jenkins2
asteriskteam at digium.com
Mon Feb 19 20:40:21 CST 2018
Jenkins2 has submitted this change and it was merged. ( https://gerrit.asterisk.org/8203 )
Change subject: optional_api: Refactor to use vector's and standard allocators.
......................................................................
optional_api: Refactor to use vector's and standard allocators.
* Replace ad-hoc array management with macro's from vector.h.
* Remove redundent logger messages.
* Use normal Asterisk allocators instead of directly using libc
allocators.
* Free memory when an API has no implementation or users.
Change-Id: Ic6ecb31798d4a78e7df39ece86a68b60eac05bf5
---
M main/optional_api.c
1 file changed, 47 insertions(+), 125 deletions(-)
Approvals:
Joshua Colp: Looks good to me, but someone else must approve
Kevin Harwell: Looks good to me, approved
Jenkins2: Approved for Submit
diff --git a/main/optional_api.c b/main/optional_api.c
index 9b9a1a0..d63129c 100644
--- a/main/optional_api.c
+++ b/main/optional_api.c
@@ -20,6 +20,7 @@
#include "asterisk/optional_api.h"
#include "asterisk/utils.h"
+#include "asterisk/vector.h"
#if defined(OPTIONAL_API)
@@ -56,15 +57,17 @@
struct optional_api {
/*! Pointer to the implementation function; could be null */
ast_optional_fn impl;
- /*! Variable length array of users of this API */
- struct optional_api_user **users;
- /*! Allocated size of the \a users array */
- size_t users_maxlen;
- /*! Number of entries in the \a users array */
- size_t users_len;
+ /*! Users of the API */
+ AST_VECTOR(, struct optional_api_user *) users;
/*! Name of the optional API function */
char symname[];
};
+
+/*! Vector of \ref optional_api functions */
+AST_VECTOR(, struct optional_api *) apis;
+
+#define USER_OPTIONAL_REF_CMP(ele, value) (ele->optional_ref == value)
+#define OPTIONAL_API_SYMNAME_CMP(ele, value) (!strcmp(ele->symname, value))
/*!
* \brief Free an \ref optional_api_user.
@@ -74,7 +77,7 @@
static void optional_api_user_destroy(struct optional_api_user *user)
{
*user->optional_ref = user->stub;
- ast_std_free(user);
+ ast_free(user);
}
/*!
@@ -93,8 +96,10 @@
struct optional_api_user *user;
size_t size = sizeof(*user) + strlen(module) + 1;
- user = ast_std_calloc(1, size);
+ user = ast_calloc(1, size);
if (!user) {
+ ast_do_crash();
+
return NULL;
}
@@ -112,17 +117,15 @@
*/
static void optional_api_destroy(struct optional_api *api)
{
- while (api->users_len--) {
- optional_api_user_destroy(api->users[api->users_len]);
- }
- ast_std_free(api->users);
- api->users = NULL;
- api->users_maxlen = 0;
- ast_std_free(api);
+ AST_VECTOR_REMOVE_CMP_UNORDERED(&apis, api,
+ AST_VECTOR_ELEM_DEFAULT_CMP, AST_VECTOR_ELEM_CLEANUP_NOOP);
+ AST_VECTOR_CALLBACK_VOID(&api->users, optional_api_user_destroy);
+ AST_VECTOR_FREE(&api->users);
+ ast_free(api);
}
/*!
- * \brief Create an \ref optional_api.
+ * \brief Create and link an \ref optional_api.
*
* \param symname Name of the optional function.
* \return New \ref optional_api.
@@ -131,12 +134,12 @@
static struct optional_api *optional_api_create(const char *symname)
{
struct optional_api *api;
- size_t size;
- size = sizeof(*api) + strlen(symname) + 1;
- api = ast_std_calloc(1, size);
- if (!api) {
- ast_log(LOG_ERROR, "Failed to allocate api\n");
+ api = ast_calloc(1, sizeof(*api) + strlen(symname) + 1);
+ if (!api || AST_VECTOR_APPEND(&apis, api)) {
+ ast_free(api);
+ ast_do_crash();
+
return NULL;
}
@@ -144,16 +147,6 @@
return api;
}
-
-/*! Array of \ref optional_api functions */
-struct {
- /*! Variable length array of API's */
- struct optional_api **list;
- /*! Allocated size of the \a list array */
- size_t maxlen;
- /*! Number of entries in the \a list array */
- size_t len;
-} apis;
/*!
* \brief Gets (or creates) the \ref optional_api for the given function.
@@ -164,43 +157,16 @@
*/
static struct optional_api *get_api(const char *symname)
{
- struct optional_api *api;
- size_t i;
+ struct optional_api **api;
/* Find one, if we already have it */
- if (apis.list) {
- for (i = 0; i < apis.len; ++i) {
- if (strcmp(symname, apis.list[i]->symname) == 0) {
- return apis.list[i];
- }
- }
+ api = AST_VECTOR_GET_CMP(&apis, symname, OPTIONAL_API_SYMNAME_CMP);
+ if (api) {
+ return *api;
}
/* API not found. Build one */
- api = optional_api_create(symname);
- if (!api) {
- return NULL;
- }
-
- /* Grow the list, if needed */
- if (apis.len + 1 > apis.maxlen) {
- size_t new_maxlen = apis.maxlen ? 2 * apis.maxlen : 1;
- struct optional_api **new_list;
-
- new_list = ast_std_realloc(apis.list, new_maxlen * sizeof(*new_list));
- if (!new_list) {
- optional_api_destroy(api);
- ast_log(LOG_ERROR, "Failed to allocate api list\n");
- return NULL;
- }
-
- apis.maxlen = new_maxlen;
- apis.list = new_list;
- }
-
- apis.list[apis.len++] = api;
-
- return api;
+ return optional_api_create(symname);
}
/*!
@@ -232,13 +198,14 @@
static void optional_api_set_impl(struct optional_api *api,
ast_optional_fn impl)
{
- size_t i;
-
api->impl = impl;
/* re-link all users */
- for (i = 0; i < api->users_len; ++i) {
- optional_api_user_relink(api->users[i], api);
+ if (AST_VECTOR_SIZE(&api->users)) {
+ AST_VECTOR_CALLBACK_VOID(&api->users, optional_api_user_relink, api);
+ } else if (!impl) {
+ /* No users or impl means we should delete this api. */
+ optional_api_destroy(api);
}
}
@@ -247,13 +214,9 @@
struct optional_api *api;
api = get_api(symname);
- if (!api) {
- ast_log(LOG_ERROR, "%s: Allocation failed\n", symname);
- ast_do_crash();
- return;
+ if (api) {
+ optional_api_set_impl(api, impl);
}
-
- optional_api_set_impl(api, impl);
}
void ast_optional_api_unprovide(const char *symname, ast_optional_fn impl)
@@ -261,13 +224,9 @@
struct optional_api *api;
api = get_api(symname);
- if (!api) {
- ast_log(LOG_ERROR, "%s: Could not find api\n", symname);
- ast_do_crash();
- return;
+ if (api) {
+ optional_api_set_impl(api, 0);
}
-
- optional_api_set_impl(api, 0);
}
void ast_optional_api_use(const char *symname, ast_optional_fn *optional_ref,
@@ -276,73 +235,36 @@
struct optional_api_user *user;
struct optional_api *api;
-
api = get_api(symname);
if (!api) {
- ast_log(LOG_ERROR, "%s: Allocation failed\n", symname);
- ast_do_crash();
return;
}
user = optional_api_user_create(optional_ref, stub, module);
if (!user) {
- ast_log(LOG_ERROR, "%s: Allocation failed\n", symname);
- ast_do_crash();
return;
}
/* Add user to the API */
- if (api->users_len + 1 > api->users_maxlen) {
- size_t new_maxlen = api->users_maxlen ? 2 * api->users_maxlen : 1;
- struct optional_api_user **new_list;
-
- new_list = ast_std_realloc(api->users, new_maxlen * sizeof(*new_list));
- if (!new_list) {
- optional_api_user_destroy(user);
- ast_log(LOG_ERROR, "Failed to allocate api list\n");
- ast_do_crash();
- return;
- }
-
- api->users_maxlen = new_maxlen;
- api->users = new_list;
+ if (!AST_VECTOR_APPEND(&api->users, user)) {
+ optional_api_user_relink(user, api);
+ } else {
+ optional_api_user_destroy(user);
+ ast_do_crash();
}
-
- api->users[api->users_len++] = user;
-
- optional_api_user_relink(user, api);
}
void ast_optional_api_unuse(const char *symname, ast_optional_fn *optional_ref,
const char *module)
{
struct optional_api *api;
- size_t i;
api = get_api(symname);
- if (!api) {
- ast_log(LOG_ERROR, "%s: Could not find api\n", symname);
- ast_do_crash();
- return;
- }
-
- for (i = 0; i < api->users_len; ++i) {
- struct optional_api_user *user = api->users[i];
-
- if (user->optional_ref == optional_ref) {
- if (*user->optional_ref != user->stub) {
- *user->optional_ref = user->stub;
- }
-
- /* Remove from the list */
- api->users[i] = api->users[--api->users_len];
-
- optional_api_user_destroy(user);
- return;
+ if (api) {
+ AST_VECTOR_REMOVE_CMP_UNORDERED(&api->users, optional_ref, USER_OPTIONAL_REF_CMP, optional_api_user_destroy);
+ if (!api->impl && !AST_VECTOR_SIZE(&api->users)) {
+ optional_api_destroy(api);
}
}
-
- ast_log(LOG_ERROR, "%s: Could not find user %s\n", symname, module);
}
-
#endif /* defined(OPTIONAL_API) */
--
To view, visit https://gerrit.asterisk.org/8203
To unsubscribe, visit https://gerrit.asterisk.org/settings
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ic6ecb31798d4a78e7df39ece86a68b60eac05bf5
Gerrit-Change-Number: 8203
Gerrit-PatchSet: 2
Gerrit-Owner: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180219/00d6eed3/attachment.html>
More information about the asterisk-code-review
mailing list