[prev in list] [next in list] [prev in thread] [next in thread]
List: asterisk-dev
Subject: [asterisk-dev] AO2 API changes for REF_DEBUG
From: Corey Farrell <git () cfware ! com>
Date: 2016-07-08 19:39:36
Message-ID: CABFe017hEyP1NoF8WGx_BCuaLtzaU2v7ChPufj88OjSUTGcV1Q () mail ! gmail ! com
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
Hello everyone,
I've opened ASTERISK-26171 to track my effort to include a "%p" pointer
(debugstorage) in the REF_DEBUG log. Providing this information to
refcounter.py allows it to remove matching ref and unref from the output.
It also allows detection of indirect leaks where one object leaked because
of another object. Examples of updated output is posted to JIRA [1],
functional code is on Gerrit [2] and [3]. I'd like to get feedback on the
API before I proceed further.
Tracking of these pointers required a new parameter to be used when
manipulating AO2 references to associate the reference with an address
(debugstorage). This does not have to be where the reference is actually
being stored and multiple references can be marked as stored at the same
pointer. Ex: ao2_link/ao2_unlink uses the container as debugstorage
instead of the node.
How should I deal with addition of 'void *debugstorage' to AO2 reference
manipulation API's? The AO2 API's we are looking at are:
__ao2_alloc, __ao2_ref
__ao2_find, __ao2_callback, __ao2_callback_data
__ao2_global_obj_ref (possibly others in the ao2_global_obj namespace)
__ao2_container_alloc_hash, __ao2_container_alloc_list,
__ao2_container_alloc_rbtree, __ao2_container_clone
ao2_iterator_init, __ao2_iterator_next
My current patch suffixes many of the existing functions with _full and
have macro's to cover the original API. After a comment from Matt Jordan I
think it would be better to just add 'void *debugstorage' parameter to the
current ABI functions. They are very infrequently used directly, so this
will be small refactor of Asterisk sources. For each of the ABI functions
listed above, do we want to create a new macro variant that supports
passing debugstorage, or do we add debugstorage to one or more of the
existing macros? My hope is for the API to encourage use of non-NULL
debugstorage where possible, without being a nuisance where it's not
possible.
Additionally should we have _s prefixed macro's? Examples from Gerrit
reviews: ao2_s_init, ao2_s_set, ao2_s_cleanup, ao2_s_replace,
ao2_s_container_alloc_hash, ast_s_format_create
The idea is that objects would use the _s API variant to set/unset ao2
objects on member fields and local variables. In all cases the _s variant
take the first parameter is a pointer to a pointer, in most cases the
pointer is written to. For example 'myobj->field = ao2_bump_full(value,
"", &myobj->field);' can be rewritten as 'ao2_s_set(&myobj->field,
value);'. Beyond the simplest ao2_s_ macro's many are implemented
indirectly using ao2_s_getter.
Links:
[1] https://issues.asterisk.org/jira/browse/ASTERISK-26171
[2] https://gerrit.asterisk.org/3141
[3] https://gerrit.asterisk.org/3161
[Attachment #5 (text/html)]
<div dir="ltr">Hello everyone,<div><br></div><div>I've opened ASTERISK-26171 to \
track my effort to include a "%p" pointer (debugstorage) in the REF_DEBUG \
log. Providing this information to refcounter.py allows it to remove matching ref \
and unref from the output. It also allows detection of indirect leaks where one \
object leaked because of another object. Examples of updated output is posted to \
JIRA [1], functional code is on Gerrit [2] and [3]. I'd like to get feedback on \
the API before I proceed further.</div><div><br></div><div>Tracking of these pointers \
required a new parameter to be used when manipulating AO2 references to associate the \
reference with an address (debugstorage). This does not have to be where the \
reference is actually being stored and multiple references can be marked as stored at \
the same pointer. Ex: ao2_link/ao2_unlink uses the container as debugstorage \
instead of the node.<br></div><div><br></div><div>How should I deal with addition of \
'void *debugstorage' to AO2 reference manipulation API's? The AO2 \
API's we are looking at are:</div>__ao2_alloc, __ao2_ref<div>__ao2_find, \
__ao2_callback, __ao2_callback_data</div><div>__ao2_global_obj_ref (possibly others \
in the ao2_global_obj namespace)<br></div><div>__ao2_container_alloc_hash, \
__ao2_container_alloc_list, __ao2_container_alloc_rbtree, \
__ao2_container_clone</div><div>ao2_iterator_init, \
__ao2_iterator_next</div><div><br></div><div>My current patch suffixes many of the \
existing functions with _full and have macro's to cover the original API. After \
a comment from Matt Jordan I think it would be better to just add 'void \
*debugstorage' parameter to the current ABI functions. They are very \
infrequently used directly, so this will be small refactor of Asterisk sources. \
For each of the ABI functions listed above, do we want to create a new macro variant \
that supports passing debugstorage, or do we add debugstorage to one or more of the \
existing macros? My hope is for the API to encourage use of non-NULL debugstorage \
where possible, without being a nuisance where it's not \
possible.</div><div><br></div><div><br></div><div><br></div><div>Additionally should \
we have _s prefixed macro's? Examples from Gerrit reviews: ao2_s_init, \
ao2_s_set, ao2_s_cleanup, ao2_s_replace, ao2_s_container_alloc_hash, \
ast_s_format_create</div><div><br></div><div>The idea is that objects would use the \
_s API variant to set/unset ao2 objects on member fields and local variables. In \
all cases the _s variant take the first parameter is a pointer to a pointer, in most \
cases the pointer is written to. For example 'myobj->field = \
ao2_bump_full(value, "", &myobj->field);' can be rewritten as \
'ao2_s_set(&myobj->field, value);'. Beyond the simplest ao2_s_ \
macro's many are implemented indirectly using \
ao2_s_getter.</div><div><br></div><div><br></div><div>Links:</div><div>[1] <a \
href="https://issues.asterisk.org/jira/browse/ASTERISK-26171">https://issues.asterisk.org/jira/browse/ASTERISK-26171</a></div><div>[2] \
<a href="https://gerrit.asterisk.org/3141">https://gerrit.asterisk.org/3141</a></div><div>[3] \
<a href="https://gerrit.asterisk.org/3161">https://gerrit.asterisk.org/3161</a><br></div></div>
--
_____________________________________________________________________
-- Bandwidth and Colocation Provided by http://www.api-digital.com --
asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
http://lists.digium.com/mailman/listinfo/asterisk-dev
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic