[Asterisk-code-review] sorcery: Add ability to insert a wizard in an object type's ... (asterisk[13])

Matt Jordan asteriskteam at digium.com
Fri May 8 07:53:06 CDT 2015


Matt Jordan has posted comments on this change.

Change subject: sorcery: Add ability to insert a wizard in an object type's list
......................................................................


Patch Set 1: Code-Review-1

(2 comments)

https://gerrit.asterisk.org/#/c/394/1/include/asterisk/sorcery.h
File include/asterisk/sorcery.h:

Line 447: /*! \brief Interface for a sorcery object type wizards */
        : AST_VECTOR_RW(ast_sorcery_object_wizards, struct ast_sorcery_object_wizard *);
Why is this vector exposed in the header? I would think that we'd want the locking semantics of the vector and its manipulation to be handled by sorcery.c, and not require external modules to be responsible for it.

If this is exposed only for the test module, then I would either:
a) Document here that it should not be accessed by users of sorcery, and is only exposed for unit test purposes.
b) Move the vector to sorcery.c, and provide TEST_FRAMEWORK defined accessors that the unit test needs.


Line 513: /*!
        :  * \brief Insert an additional object wizard mapping at a specific position
        :  * in the wizard list
        :  *
        :  * \param sorcery Pointer to a sorcery structure
        :  * \param type Type of object to apply to
        :  * \param module The name of the module, typically AST_MODULE
        :  * \param name Name of the wizard to use
        :  * \param data Data to be passed to wizard
        :  * \param caching Wizard should cache
        :  * \param position An index to inser to or one of ast_sorcery_wizard_position
        :  *
        :  * \return What occurred when applying the mapping
        :  *
        :  * \note This should be called *after* applying default mappings
        :  */
Since this is going into 13, having a \since tag on these new functions would be nice.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9d2469a9296b2698082c0989e25e6848dc403b57
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: George Joseph <george.joseph at fairview5.com>
Gerrit-Reviewer: Matt Jordan <mjordan at digium.com>
Gerrit-HasComments: Yes



More information about the asterisk-code-review mailing list