From fec6e1bd87cb32682b78a62f745ec936df2f2ae5 Mon Sep 17 00:00:00 2001 From: Corey Farrell Date: Sun, 22 Sep 2019 17:59:54 -0400 Subject: [PATCH] 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 --- include/asterisk/astobj2.h | 5 ++--- main/astobj2.c | 22 +++++++++++----------- main/astobj2_container.c | 24 ++++++++++++------------ main/astobj2_hash.c | 21 +++++++++++---------- main/astobj2_rbtree.c | 13 +++++++------ 5 files changed, 43 insertions(+), 42 deletions(-) diff --git a/include/asterisk/astobj2.h b/include/asterisk/astobj2.h index af174100f6..ad8ffd8ba2 100644 --- a/include/asterisk/astobj2.h +++ b/include/asterisk/astobj2.h @@ -376,6 +376,8 @@ enum ao2_alloc_opts { * 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 @@ enum ao2_alloc_opts { * - 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 @@ void *__ao2_alloc_with_lockobj(size_t data_size, ao2_destructor_fn destructor_fn * 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 ebc07200dd..fb5bd0e6fa 100644 --- a/main/astobj2.c +++ b/main/astobj2.c @@ -67,7 +67,7 @@ struct __priv_data { * \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 @@ struct __priv_data { * 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 @@ int __ao2_ref(void *user_data, int delta, __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 @@ int __ao2_ref(void *user_data, int delta, 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 @@ int __ao2_ref(void *user_data, int delta, 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 @@ static void *internal_ao2_alloc(size_t data_size, ao2_destructor_fn destructor_f 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 9a837bde75..0b580e6389 100644 --- a/main/astobj2_container.c +++ b/main/astobj2_container.c @@ -70,7 +70,7 @@ int __container_unlink_node_debug(struct ao2_container_node *node, uint32_t flag 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 @@ int __ao2_link(struct ao2_container *self, void *obj_new, int flags, res = 1; break; case AO2_CONTAINER_INSERT_NODE_REJECTED: - ao2_t_ref(node, -1, NULL); + ao2_ref(node, -1); break; } } @@ -386,7 +386,7 @@ static void *internal_ao2_traverse(struct ao2_container *self, enum search_flags } 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 @@ void ao2_iterator_restart(struct ao2_iterator *iter) 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 @@ void *__ao2_iterator_next(struct ao2_iterator *iter, __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 @@ void *__ao2_iterator_next(struct ao2_iterator *iter, /* 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 @@ static int dup_obj_cb(void *obj, void *arg, int flags) { 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 @@ int ao2_container_dup(struct ao2_container *dest, struct ao2_container *src, enu 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 @@ static int dup_weakproxy_cb(void *proxy, void *arg, int flags) 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 @@ int ao2_container_dup_weakproxy_objs(struct ao2_container *dest, struct ao2_cont 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 55077c5d34..c24b8a0b93 100644 --- a/main/astobj2_hash.c +++ b/main/astobj2_hash.c @@ -210,7 +210,8 @@ static struct hash_bucket_node *hash_ao2_new_node(struct ao2_container_hash *sel 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 @@ static enum ao2_container_insert hash_ao2_insert_node(struct ao2_container_hash 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 @@ static enum ao2_container_insert hash_ao2_insert_node(struct ao2_container_hash 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 @@ static struct hash_bucket_node *hash_ao2_find_first(struct ao2_container_hash *s } /* We have the first traversal node */ - ao2_t_ref(node, +1, NULL); + ao2_ref(node, +1); return node; } } @@ -457,7 +458,7 @@ static struct hash_bucket_node *hash_ao2_find_first(struct ao2_container_hash *s } /* We have the first traversal node */ - ao2_t_ref(node, +1, NULL); + ao2_ref(node, +1); return node; } } @@ -526,7 +527,7 @@ static struct hash_bucket_node *hash_ao2_find_next(struct ao2_container_hash *se } /* 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 @@ static struct hash_bucket_node *hash_ao2_find_next(struct ao2_container_hash *se * 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 @@ hash_descending_resume:; } /* 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 @@ hash_descending_resume:; * 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 @@ hash_ascending_resume:; } /* 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 73d689664e..d7c76e676d 100644 --- a/main/astobj2_rbtree.c +++ b/main/astobj2_rbtree.c @@ -905,7 +905,8 @@ static struct rbtree_node *rb_ao2_new_node(struct ao2_container_rbtree *self, vo { 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 @@ static enum ao2_container_insert rb_ao2_insert_node(struct ao2_container_rbtree 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 @@ static struct rbtree_node *rb_ao2_find_next(struct ao2_container_rbtree *self, s } /* 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 @@ static struct rbtree_node *rb_ao2_find_next(struct ao2_container_rbtree *self, s * 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 @@ static struct rbtree_node *rb_ao2_find_next(struct ao2_container_rbtree *self, s } /* 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 @@ static struct rbtree_node *rb_ao2_find_first(struct ao2_container_rbtree *self, } /* We have the first traversal node */ - ao2_t_ref(node, +1, NULL); + ao2_ref(node, +1); return node; }