[Asterisk-code-review] core: Add AO2_ALLOC_OPT_NO_REF_DEBUG option. (...asterisk[16])
George Joseph
asteriskteam at digium.com
Wed Sep 25 06:03:47 CDT 2019
George Joseph has submitted this change and it was merged. ( https://gerrit.asterisk.org/c/asterisk/+/12928 )
Change subject: core: Add AO2_ALLOC_OPT_NO_REF_DEBUG option.
......................................................................
core: Add AO2_ALLOC_OPT_NO_REF_DEBUG option.
Previous to this patch passing a NULL tag to ao2_alloc or ao2_ref based
functions would result in the reference not being logged under
REF_DEBUG. This could sometimes cause inaccurate logging if NULL was
accidentally passed to a reference action. Now reference logging is
only disabled by option passed to the allocation method.
Change-Id: I3c17d867d901d53f9fcd512bef4d52e342637b54
---
M include/asterisk/astobj2.h
M main/astobj2.c
M main/astobj2_container.c
M main/astobj2_hash.c
M main/astobj2_rbtree.c
5 files changed, 43 insertions(+), 42 deletions(-)
Approvals:
Richard Mudgett: Looks good to me, but someone else must approve
George Joseph: Looks good to me, approved; Approved for Submit
diff --git a/include/asterisk/astobj2.h b/include/asterisk/astobj2.h
index af17410..ad8ffd8 100644
--- a/include/asterisk/astobj2.h
+++ b/include/asterisk/astobj2.h
@@ -376,6 +376,8 @@
* should never be passed directly to ao2_alloc.
*/
AO2_ALLOC_OPT_LOCK_OBJ = AO2_ALLOC_OPT_LOCK_MASK,
+ /*! The ao2 object will not record any REF_DEBUG entries */
+ AO2_ALLOC_OPT_NO_REF_DEBUG = (1 << 2),
};
/*!
@@ -396,8 +398,6 @@
* - the returned pointer cannot be free()'d or realloc()'ed;
* rather, we just call ao2_ref(o, -1);
*
- * \note refdebug logging is skipped if debug_msg is NULL
- *
* @{
*/
@@ -457,7 +457,6 @@
* can go away is when we release our reference, and it is
* the last one in existence.
*
- * \note refdebug logging is skipped if tag is NULL
* @{
*/
diff --git a/main/astobj2.c b/main/astobj2.c
index ebc0720..fb5bd0e 100644
--- a/main/astobj2.c
+++ b/main/astobj2.c
@@ -67,7 +67,7 @@
* \note This field is constant after object creation. It shares
* a uint32_t with \ref lockused and \ref magic.
*/
- uint32_t options:2;
+ uint32_t options:3;
/*!
* \brief Set to 1 when the lock is used if refdebug is enabled.
*
@@ -90,11 +90,11 @@
* all bitfields into a single 'uint32_t flags' field and use
* atomic operations from \file lock.h to perform writes.
*/
- uint32_t magic:29;
+ uint32_t magic:28;
};
-#define AO2_MAGIC 0x1a70b123
-#define AO2_WEAK 0x1a70b122
+#define AO2_MAGIC 0xa70b123
+#define AO2_WEAK 0xa70b122
#define IS_AO2_MAGIC_BAD(p) (AO2_MAGIC != (p->priv_data.magic | 1))
/*!
@@ -585,10 +585,10 @@
__ast_assert_failed(0, excessive_ref_buf, file, line, func);
}
- if (ref_log && tag) {
+ if (ref_log && !(obj->priv_data.options & AO2_ALLOC_OPT_NO_REF_DEBUG)) {
fprintf(ref_log, "%p,%s%d,%d,%s,%d,%s,%d,%s\n", user_data,
(delta < 0 ? "" : "+"), delta, ast_get_tid(),
- file, line, func, (int)ret, tag);
+ file, line, func, (int)ret, tag ?: "");
fflush(ref_log);
}
return ret;
@@ -599,7 +599,7 @@
ast_log(__LOG_ERROR, file, line, func,
"Invalid refcount %d on ao2 object %p\n", (int)current_value, user_data);
if (ref_log) {
- /* Log to ref_log invalid even if (tag == NULL) */
+ /* Log to ref_log even if AO2_ALLOC_OPT_NO_REF_DEBUG */
fprintf(ref_log, "%p,%d,%d,%s,%d,%s,**invalid**,%s\n",
user_data, delta, ast_get_tid(), file, line, func, tag ?: "");
fflush(ref_log);
@@ -655,9 +655,9 @@
break;
}
- if (ref_log && tag) {
+ if (ref_log && !(obj->priv_data.options & AO2_ALLOC_OPT_NO_REF_DEBUG)) {
fprintf(ref_log, "%p,%d,%d,%s,%d,%s,**destructor**lock-state:%s**,%s\n",
- user_data, delta, ast_get_tid(), file, line, func, lock_state, tag);
+ user_data, delta, ast_get_tid(), file, line, func, lock_state, tag ?: "");
fflush(ref_log);
}
@@ -752,9 +752,9 @@
ast_atomic_fetchadd_int(&ao2.total_refs, 1);
#endif
- if (ref_log && tag) {
+ if (ref_log && !(obj->priv_data.options & AO2_ALLOC_OPT_NO_REF_DEBUG)) {
fprintf(ref_log, "%p,+1,%d,%s,%d,%s,**constructor**%zu**%zu**,%s\n",
- EXTERNAL_OBJ(obj), ast_get_tid(), file, line, func, overhead, data_size, tag);
+ EXTERNAL_OBJ(obj), ast_get_tid(), file, line, func, overhead, data_size, tag ?: "");
fflush(ref_log);
}
diff --git a/main/astobj2_container.c b/main/astobj2_container.c
index 9a837bd..0b580e6 100644
--- a/main/astobj2_container.c
+++ b/main/astobj2_container.c
@@ -70,7 +70,7 @@
if (flags & AO2_UNLINK_NODE_UNREF_NODE) {
/* Remove node from container */
- ao2_t_ref(node, -1, NULL);
+ ao2_ref(node, -1);
}
return 1;
@@ -146,7 +146,7 @@
res = 1;
break;
case AO2_CONTAINER_INSERT_NODE_REJECTED:
- ao2_t_ref(node, -1, NULL);
+ ao2_ref(node, -1);
break;
}
}
@@ -386,7 +386,7 @@
}
if (node) {
/* Unref the node from self->v_table->traverse_first/traverse_next() */
- ao2_t_ref(node, -1, NULL);
+ ao2_ref(node, -1);
}
if (flags & OBJ_NOLOCK) {
@@ -517,7 +517,7 @@
ao2_rdlock(iter->c);
}
- ao2_t_ref(iter->last_node, -1, NULL);
+ ao2_ref(iter->last_node, -1);
iter->last_node = NULL;
if (iter->flags & AO2_ITERATOR_DONTLOCK) {
@@ -604,7 +604,7 @@
__ao2_ref(ret, +1, tag ?: "Next iterator object.", file, line, func);
/* Bump the container's node ref for the iterator. */
- ao2_t_ref(node, +1, NULL);
+ ao2_ref(node, +1);
}
} else {
/* The iteration has completed. */
@@ -614,7 +614,7 @@
/* Replace the iterator's node */
if (iter->last_node) {
- ao2_t_ref(iter->last_node, -1, NULL);
+ ao2_ref(iter->last_node, -1);
}
iter->last_node = node;
@@ -667,7 +667,7 @@
{
struct ao2_container *dest = arg;
- return ao2_t_link_flags(dest, obj, OBJ_NOLOCK, NULL) ? 0 : (CMP_MATCH | CMP_STOP);
+ return ao2_link_flags(dest, obj, OBJ_NOLOCK) ? 0 : (CMP_MATCH | CMP_STOP);
}
int ao2_container_dup(struct ao2_container *dest, struct ao2_container *src, enum search_flags flags)
@@ -685,8 +685,8 @@
ao2_t_ref(obj, -1, "Failed to put this object into the dest container.");
/* Remove all items from the dest container. */
- ao2_t_callback(dest, OBJ_NOLOCK | OBJ_UNLINK | OBJ_NODATA | OBJ_MULTIPLE, NULL,
- NULL, NULL);
+ ao2_callback(dest, OBJ_NOLOCK | OBJ_UNLINK | OBJ_NODATA | OBJ_MULTIPLE, NULL,
+ NULL);
res = -1;
}
if (!(flags & OBJ_NOLOCK)) {
@@ -717,7 +717,7 @@
return 0;
}
- ret = ao2_t_link_flags(dest, obj, OBJ_NOLOCK, NULL) ? 0 : (CMP_MATCH | CMP_STOP);
+ ret = ao2_link_flags(dest, obj, OBJ_NOLOCK) ? 0 : (CMP_MATCH | CMP_STOP);
ao2_ref(obj, -1);
return ret;
@@ -738,8 +738,8 @@
ao2_t_ref(obj, -1, "Failed to put this object into the dest container.");
/* Remove all items from the dest container. */
- ao2_t_callback(dest, OBJ_NOLOCK | OBJ_UNLINK | OBJ_NODATA | OBJ_MULTIPLE, NULL,
- NULL, NULL);
+ ao2_callback(dest, OBJ_NOLOCK | OBJ_UNLINK | OBJ_NODATA | OBJ_MULTIPLE, NULL,
+ NULL);
res = -1;
}
if (!(flags & OBJ_NOLOCK)) {
diff --git a/main/astobj2_hash.c b/main/astobj2_hash.c
index 55077c5..c24b8a0 100644
--- a/main/astobj2_hash.c
+++ b/main/astobj2_hash.c
@@ -210,7 +210,8 @@
struct hash_bucket_node *node;
int i;
- node = ao2_t_alloc_options(sizeof(*node), hash_ao2_node_destructor, AO2_ALLOC_OPT_LOCK_NOLOCK, NULL);
+ node = ao2_alloc_options(sizeof(*node), hash_ao2_node_destructor,
+ AO2_ALLOC_OPT_LOCK_NOLOCK | AO2_ALLOC_OPT_NO_REF_DEBUG);
if (!node) {
return NULL;
}
@@ -274,7 +275,7 @@
break;
case AO2_CONTAINER_ALLOC_OPT_DUPS_REPLACE:
SWAP(cur->common.obj, node->common.obj);
- ao2_t_ref(node, -1, NULL);
+ ao2_ref(node, -1);
return AO2_CONTAINER_INSERT_NODE_OBJ_REPLACED;
}
}
@@ -307,7 +308,7 @@
break;
case AO2_CONTAINER_ALLOC_OPT_DUPS_REPLACE:
SWAP(cur->common.obj, node->common.obj);
- ao2_t_ref(node, -1, NULL);
+ ao2_ref(node, -1);
return AO2_CONTAINER_INSERT_NODE_OBJ_REPLACED;
}
}
@@ -415,7 +416,7 @@
}
/* We have the first traversal node */
- ao2_t_ref(node, +1, NULL);
+ ao2_ref(node, +1);
return node;
}
}
@@ -457,7 +458,7 @@
}
/* We have the first traversal node */
- ao2_t_ref(node, +1, NULL);
+ ao2_ref(node, +1);
return node;
}
}
@@ -526,7 +527,7 @@
}
/* We have the next traversal node */
- ao2_t_ref(node, +1, NULL);
+ ao2_ref(node, +1);
/*
* Dereferencing the prev node may result in our next node
@@ -534,7 +535,7 @@
* the container uses RW locks and the container was read
* locked.
*/
- ao2_t_ref(prev, -1, NULL);
+ ao2_ref(prev, -1);
if (node->common.obj) {
return node;
}
@@ -570,7 +571,7 @@
}
/* We have the next traversal node */
- ao2_t_ref(node, +1, NULL);
+ ao2_ref(node, +1);
/*
* Dereferencing the prev node may result in our next node
@@ -578,7 +579,7 @@
* the container uses RW locks and the container was read
* locked.
*/
- ao2_t_ref(prev, -1, NULL);
+ ao2_ref(prev, -1);
if (node->common.obj) {
return node;
}
@@ -590,7 +591,7 @@
}
/* No more nodes in the container left to traverse. */
- ao2_t_ref(prev, -1, NULL);
+ ao2_ref(prev, -1);
return NULL;
}
diff --git a/main/astobj2_rbtree.c b/main/astobj2_rbtree.c
index 73d6896..d7c76e6 100644
--- a/main/astobj2_rbtree.c
+++ b/main/astobj2_rbtree.c
@@ -905,7 +905,8 @@
{
struct rbtree_node *node;
- node = ao2_t_alloc_options(sizeof(*node), rb_ao2_node_destructor, AO2_ALLOC_OPT_LOCK_NOLOCK, NULL);
+ node = ao2_alloc_options(sizeof(*node), rb_ao2_node_destructor,
+ AO2_ALLOC_OPT_LOCK_NOLOCK | AO2_ALLOC_OPT_NO_REF_DEBUG);
if (!node) {
return NULL;
}
@@ -1243,7 +1244,7 @@
break;
case AO2_CONTAINER_ALLOC_OPT_DUPS_REPLACE:
SWAP(cur->common.obj, node->common.obj);
- ao2_t_ref(node, -1, NULL);
+ ao2_ref(node, -1);
return AO2_CONTAINER_INSERT_NODE_OBJ_REPLACED;
}
@@ -1313,7 +1314,7 @@
}
/* We have the next traversal node */
- ao2_t_ref(node, +1, NULL);
+ ao2_ref(node, +1);
/*
* Dereferencing the prev node may result in our next node
@@ -1321,7 +1322,7 @@
* the container uses RW locks and the container was read
* locked.
*/
- ao2_t_ref(prev, -1, NULL);
+ ao2_ref(prev, -1);
if (node->common.obj) {
return node;
}
@@ -1329,7 +1330,7 @@
}
/* No more nodes in the container left to traverse. */
- ao2_t_ref(prev, -1, NULL);
+ ao2_ref(prev, -1);
return NULL;
}
@@ -1612,7 +1613,7 @@
}
/* We have the first traversal node */
- ao2_t_ref(node, +1, NULL);
+ ao2_ref(node, +1);
return node;
}
--
To view, visit https://gerrit.asterisk.org/c/asterisk/+/12928
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings
Gerrit-Project: asterisk
Gerrit-Branch: 16
Gerrit-Change-Id: I3c17d867d901d53f9fcd512bef4d52e342637b54
Gerrit-Change-Number: 12928
Gerrit-PatchSet: 1
Gerrit-Owner: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20190925/61948e6c/attachment-0001.html>
More information about the asterisk-code-review
mailing list