[Asterisk-code-review] ao2 container: Add support for transparent storage of weakpr... (asterisk[master])

Corey Farrell asteriskteam at digium.com
Tue Sep 6 09:33:33 CDT 2016


Corey Farrell has posted comments on this change.

Change subject: ao2_container: Add support for transparent storage of weakproxy objects.
......................................................................


Patch Set 2: Code-Review-1

Richard,

Would love you're opinion on this patch.  I'm not really happy with the API but I'm not sure how to fix.

Goals:
1) Simplify callers of ao2_find / unify code for retrieving from weakproxy in container.
2) Automate weakproxy subscription for ao2_link.
3) Avoid taking the reference bump/release of node->obj, directly return a reference to ao2_weakproxy_get_object(node->obj).


Questions/Concerns:
1) The need to check ao2_weakproxy_ref_object() in the named_locks_cmp is ugly.  I'd love if this could just be handled by internal_ao2_traverse, but I'm not sure a good way to abort CMP_MATCH in the case where weakproxy has no object.
2) Do we want this to be an alloc option, or flags given to ao2_link, ao2_find and ao2_callback?
3) How to deal with manual ao2_unlink?
4) How to deal with shutdown (cleanup of non-empty container)?
5) Do we need to deal with the possibility that a weakproxy can be associated with a new object before node_obj_proxy_cb() is called?  Current patch does not support reuse of weakproxy objects added to container due to this.

-- 
To view, visit https://gerrit.asterisk.org/3749
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I48f2dfb85e97fd0a382a869cd084afda129c95d9
Gerrit-PatchSet: 2
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-HasComments: No



More information about the asterisk-code-review mailing list