[asterisk-commits] russell: trunk r330273 - in /trunk: channels/ include/asterisk/ main/ tests/
SVN commits to the Asterisk project
asterisk-commits at lists.digium.com
Fri Jul 29 14:34:44 CDT 2011
Author: russell
Date: Fri Jul 29 14:34:36 2011
New Revision: 330273
URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=330273
Log:
astobj2: Avoid using temporary objects + ao2_find() with OBJ_POINTER.
There is a fairly common pattern making its way through the code base where we
put a temporary object on the stack so we can call ao2_find() with OBJ_POINTER.
The purpose is so that it can be passed into the object hash function.
However, this really seems like a hack and potentially error prone. This patch
is a first stab at approach to avoid having to do that.
It adds a new flag, OBJ_KEY, which can be used instead of OBJ_POINTER in these
situations. Then, the hash function can know whether it was given an object or
some custom data to hash.
The patch also changes some uses of ao2_find() for iax2_user and iax2_peer
objects to reflect how OBJ_KEY would be used.
So long, and thanks for all the fish.
Review: https://reviewboard.asterisk.org/r/1184/
Modified:
trunk/channels/chan_iax2.c
trunk/include/asterisk/astobj2.h
trunk/main/astobj2.c
trunk/tests/test_astobj2.c
Modified: trunk/channels/chan_iax2.c
URL: http://svnview.digium.com/svn/asterisk/trunk/channels/chan_iax2.c?view=diff&rev=330273&r1=330272&r2=330273
==============================================================================
--- trunk/channels/chan_iax2.c (original)
+++ trunk/channels/chan_iax2.c Fri Jul 29 14:34:36 2011
@@ -1726,8 +1726,9 @@
static int peer_hash_cb(const void *obj, const int flags)
{
const struct iax2_peer *peer = obj;
-
- return ast_str_hash(peer->name);
+ const char *name = obj;
+
+ return ast_str_hash(flags & OBJ_KEY ? name : peer->name);
}
/*!
@@ -1736,8 +1737,10 @@
static int peer_cmp_cb(void *obj, void *arg, int flags)
{
struct iax2_peer *peer = obj, *peer2 = arg;
-
- return !strcmp(peer->name, peer2->name) ? CMP_MATCH | CMP_STOP : 0;
+ const char *name = arg;
+
+ return !strcmp(peer->name, flags & OBJ_KEY ? name : peer2->name) ?
+ CMP_MATCH | CMP_STOP : 0;
}
/*!
@@ -1746,8 +1749,9 @@
static int user_hash_cb(const void *obj, const int flags)
{
const struct iax2_user *user = obj;
-
- return ast_str_hash(user->name);
+ const char *name = obj;
+
+ return ast_str_hash(flags & OBJ_KEY ? name : user->name);
}
/*!
@@ -1756,8 +1760,10 @@
static int user_cmp_cb(void *obj, void *arg, int flags)
{
struct iax2_user *user = obj, *user2 = arg;
-
- return !strcmp(user->name, user2->name) ? CMP_MATCH | CMP_STOP : 0;
+ const char *name = arg;
+
+ return !strcmp(user->name, flags & OBJ_KEY ? name : user2->name) ?
+ CMP_MATCH | CMP_STOP : 0;
}
/*!
@@ -1767,11 +1773,8 @@
static struct iax2_peer *find_peer(const char *name, int realtime)
{
struct iax2_peer *peer = NULL;
- struct iax2_peer tmp_peer = {
- .name = name,
- };
-
- peer = ao2_find(peers, &tmp_peer, OBJ_POINTER);
+
+ peer = ao2_find(peers, name, OBJ_KEY);
/* Now go for realtime if applicable */
if(!peer && realtime)
@@ -1794,11 +1797,7 @@
static struct iax2_user *find_user(const char *name)
{
- struct iax2_user tmp_user = {
- .name = name,
- };
-
- return ao2_find(users, &tmp_user, OBJ_POINTER);
+ return ao2_find(users, name, OBJ_KEY);
}
static inline struct iax2_user *user_ref(struct iax2_user *user)
{
@@ -1854,11 +1853,8 @@
/* Decrement AUTHREQ count if needed */
if (ast_test_flag64(pvt, IAX_MAXAUTHREQ)) {
struct iax2_user *user;
- struct iax2_user tmp_user = {
- .name = pvt->username,
- };
-
- user = ao2_find(users, &tmp_user, OBJ_POINTER);
+
+ user = ao2_find(users, pvt->username, OBJ_KEY);
if (user) {
ast_atomic_fetchadd_int(&user->curauthreq, -1);
user_unref(user);
@@ -6943,12 +6939,9 @@
p = find_peer(a->argv[2], 1);
if (p) {
if (p->expire > 0) {
- struct iax2_peer tmp_peer = {
- .name = a->argv[2],
- };
struct iax2_peer *peer;
- peer = ao2_find(peers, &tmp_peer, OBJ_POINTER);
+ peer = ao2_find(peers, a->argv[2], OBJ_KEY);
if (peer) {
expire_registry(peer_ref(peer)); /* will release its own reference when done */
peer_unref(peer); /* ref from ao2_find() */
@@ -7887,11 +7880,9 @@
/* If an AUTHREQ restriction is in place, make sure we can send an AUTHREQ back */
if (ast_test_flag64(p, IAX_MAXAUTHREQ)) {
- struct iax2_user *user, tmp_user = {
- .name = p->username,
- };
-
- user = ao2_find(users, &tmp_user, OBJ_POINTER);
+ struct iax2_user *user;
+
+ user = ao2_find(users, p->username, OBJ_KEY);
if (user) {
if (user->curauthreq == user->maxauthreq)
authreq_restrict = 1;
@@ -7937,14 +7928,12 @@
char rsasecret[256] = "";
int res = -1;
int x;
- struct iax2_user *user, tmp_user = {
- .name = p->username,
- };
+ struct iax2_user *user;
if (p->authrej) {
return res;
}
- user = ao2_find(users, &tmp_user, OBJ_POINTER);
+ user = ao2_find(users, p->username, OBJ_KEY);
if (user) {
if (ast_test_flag64(p, IAX_MAXAUTHREQ)) {
ast_atomic_fetchadd_int(&user->curauthreq, -1);
@@ -12400,12 +12389,9 @@
int maskfound = 0;
int found = 0;
int firstpass = 1;
- struct iax2_peer tmp_peer = {
- .name = name,
- };
if (!temponly) {
- peer = ao2_find(peers, &tmp_peer, OBJ_POINTER);
+ peer = ao2_find(peers, name, OBJ_KEY);
if (peer && !ast_test_flag64(peer, IAX_DELME))
firstpass = 0;
}
@@ -12697,12 +12683,9 @@
int oldcurauthreq = 0;
char *varname = NULL, *varval = NULL;
struct ast_variable *tmpvar = NULL;
- struct iax2_user tmp_user = {
- .name = name,
- };
if (!temponly) {
- user = ao2_find(users, &tmp_user, OBJ_POINTER);
+ user = ao2_find(users, name, OBJ_KEY);
if (user && !ast_test_flag64(user, IAX_DELME))
firstpass = 0;
}
Modified: trunk/include/asterisk/astobj2.h
URL: http://svnview.digium.com/svn/asterisk/trunk/include/asterisk/astobj2.h?view=diff&rev=330273&r1=330272&r2=330273
==============================================================================
--- trunk/include/asterisk/astobj2.h (original)
+++ trunk/include/asterisk/astobj2.h Fri Jul 29 14:34:36 2011
@@ -680,6 +680,16 @@
* by another mechanism other that the internal ao2_lock.
*/
OBJ_NOLOCK = (1 << 5),
+ /*!
+ * \brief The data is hashable, but is not an object.
+ *
+ * This can be used when you want to be able to pass custom data
+ * to a hash function that is not a full object, but perhaps just
+ * a string.
+ *
+ * \note OBJ_KEY and OBJ_POINTER are mutually exclusive options.
+ */
+ OBJ_KEY = (1 << 6),
};
/*!
@@ -964,9 +974,9 @@
#endif
-void *__ao2_find_debug(struct ao2_container *c, void *arg, enum search_flags flags, char *tag,
+void *__ao2_find_debug(struct ao2_container *c, const void *arg, enum search_flags flags, char *tag,
char *file, int line, const char *funcname);
-void *__ao2_find(struct ao2_container *c, void *arg, enum search_flags flags);
+void *__ao2_find(struct ao2_container *c, const void *arg, enum search_flags flags);
/*! \brief
*
Modified: trunk/main/astobj2.c
URL: http://svnview.digium.com/svn/asterisk/trunk/main/astobj2.c?view=diff&rev=330273&r1=330272&r2=330273
==============================================================================
--- trunk/main/astobj2.c (original)
+++ trunk/main/astobj2.c Fri Jul 29 14:34:36 2011
@@ -648,8 +648,8 @@
* run the hash function. Otherwise, scan the whole container
* (this only for the time being. We need to optimize this.)
*/
- if ((flags & OBJ_POINTER)) /* we know hash can handle this case */
- start = i = c->hash_fn(arg, flags & OBJ_POINTER) % c->n_buckets;
+ if ((flags & (OBJ_POINTER | OBJ_KEY))) /* we know hash can handle this case */
+ start = i = c->hash_fn(arg, flags & (OBJ_POINTER | OBJ_KEY)) % c->n_buckets;
else /* don't know, let's scan all buckets */
start = i = -1; /* XXX this must be fixed later. */
@@ -801,14 +801,14 @@
/*!
* the find function just invokes the default callback with some reasonable flags.
*/
-void *__ao2_find_debug(struct ao2_container *c, void *arg, enum search_flags flags, char *tag, char *file, int line, const char *funcname)
-{
- return __ao2_callback_debug(c, flags, c->cmp_fn, arg, tag, file, line, funcname);
-}
-
-void *__ao2_find(struct ao2_container *c, void *arg, enum search_flags flags)
-{
- return __ao2_callback(c, flags, c->cmp_fn, arg);
+void *__ao2_find_debug(struct ao2_container *c, const void *arg, enum search_flags flags, char *tag, char *file, int line, const char *funcname)
+{
+ return __ao2_callback_debug(c, flags, c->cmp_fn, (void *) arg, tag, file, line, funcname);
+}
+
+void *__ao2_find(struct ao2_container *c, const void *arg, enum search_flags flags)
+{
+ return __ao2_callback(c, flags, c->cmp_fn, (void *) arg);
}
/*!
Modified: trunk/tests/test_astobj2.c
URL: http://svnview.digium.com/svn/asterisk/trunk/tests/test_astobj2.c?view=diff&rev=330273&r1=330272&r2=330273
==============================================================================
--- trunk/tests/test_astobj2.c (original)
+++ trunk/tests/test_astobj2.c Fri Jul 29 14:34:36 2011
@@ -38,7 +38,6 @@
#include "asterisk/astobj2.h"
struct test_obj {
- char c[20];
int i;
int *destructor_count;
};
@@ -75,20 +74,35 @@
static int test_cmp_cb(void *obj, void *arg, int flags)
{
struct test_obj *cmp_obj = (struct test_obj *) obj;
- struct test_obj *test_obj = (struct test_obj *) arg;
+
if (!arg) {
return 0;
}
- return (cmp_obj->i == test_obj->i) ? CMP_MATCH | CMP_STOP : 0;
+
+ if (flags & OBJ_KEY) {
+ int *i = (int *) arg;
+ return (cmp_obj->i == *i) ? CMP_MATCH | CMP_STOP : 0;
+ } else {
+ struct test_obj *test_obj = (struct test_obj *) arg;
+ return (cmp_obj->i == test_obj->i) ? CMP_MATCH | CMP_STOP : 0;
+ }
}
static int test_hash_cb(const void *obj, const int flags)
{
- struct test_obj *test_obj = (struct test_obj *) obj;
- if (!test_obj || ast_strlen_zero(test_obj->c)) {
+ if (!obj) {
return 0;
}
- return ast_str_hash(test_obj->c);
+
+ if (flags & OBJ_KEY) {
+ const int *i = obj;
+
+ return *i;
+ } else {
+ const struct test_obj *test_obj = obj;
+
+ return test_obj->i;
+ }
}
static int astobj2_test_helper(int use_hash, int use_cmp, unsigned int lim, struct ast_test *test)
@@ -128,7 +142,6 @@
res = AST_TEST_FAIL;
goto cleanup;
}
- snprintf(obj->c, sizeof(obj->c), "zombie #%d", num);
obj->destructor_count = &destructor_count;
obj->i = num;
ao2_link(c1, obj);
@@ -163,11 +176,27 @@
num = 75;
for (; num; num--) {
int i = (ast_random() % ((lim / 2)) + 1); /* find a random object */
- snprintf(tmp_obj.c, sizeof(tmp_obj.c), "zombie #%d", i);
tmp_obj.i = i;
if (!(obj = ao2_find(c1, &tmp_obj, OBJ_POINTER))) {
res = AST_TEST_FAIL;
ast_test_status_update(test, "COULD NOT FIND:%d, ao2_find() with OBJ_POINTER flag failed.\n", i);
+ } else {
+ /* a correct match will only take place when the custom cmp function is used */
+ if (use_cmp && obj->i != i) {
+ ast_test_status_update(test, "object %d does not match object %d\n", obj->i, tmp_obj.i);
+ res = AST_TEST_FAIL;
+ }
+ ao2_t_ref(obj, -1, "test");
+ }
+ }
+
+ /* Testing ao2_find with OBJ_KEY */
+ num = 75;
+ for (; num; num--) {
+ int i = (ast_random() % ((lim / 2)) + 1); /* find a random object */
+ if (!(obj = ao2_find(c1, &i, OBJ_KEY))) {
+ res = AST_TEST_FAIL;
+ ast_test_status_update(test, "COULD NOT FIND:%d, ao2_find() with OBJ_KEY flag failed.\n", i);
} else {
/* a correct match will only take place when the custom cmp function is used */
if (use_cmp && obj->i != i) {
@@ -374,7 +403,7 @@
int num;
static const int NUM_OBJS = 5;
int destructor_count = NUM_OBJS;
- struct test_obj tmp_obj = { "", };
+ struct test_obj tmp_obj = { 0, };
switch (cmd) {
case TEST_INIT:
More information about the asterisk-commits
mailing list