[asterisk-dev] [Code Review] 2585: Sorcery Extended Fields and Opaque Data

[Mark Michelson]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/2585/#review8896
-----------------------------------------------------------



/trunk/include/asterisk/sorcery.h
<https://reviewboard.asterisk.org/r/2585/#comment17466>

    This should document what happens when attempting to set an object field whose name matches an existing one.



/trunk/main/sorcery.c
<https://reviewboard.asterisk.org/r/2585/#comment17467>

    I've been contemplating this. Do you think there could be value in allowing an object to specify that it does not allow extended fields?



/trunk/main/sorcery.c
<https://reviewboard.asterisk.org/r/2585/#comment17464>

    Got a red blob here.



/trunk/main/sorcery.c
<https://reviewboard.asterisk.org/r/2585/#comment17465>

    I disagree with the behavior here. I think that when setting an extended object field, we should not return an error if a field with the given name already exists. I think we should instead overwrite the previous value with the new one.
    
    Why? Consider the scenario of people writing a template with common default options for a particular object. The majority of objects that use this template will just add new options on, but some might wish to override the default value on the template. (This came up in a discussion Richard and I had when I was trying to figure out how to handle duplicate items for features config)


- Mark Michelson


On May 31, 2013, 8:11 p.m., Joshua Colp wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2585/
> -----------------------------------------------------------
> 
> (Updated May 31, 2013, 8:11 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> This change adds two new things:
> 
> 1. Extended fields
> 
> Extended fields are fields specifically marked as extending an object. This means they are not held to the same type safety as normal fields and simply exist as a list that can be queried. The pro of this is that the object that is extended does *not* have to be modified when other modules need to attach data to it. The con of this is that type safety must be enforced at the time of consumption by the user of the extended field. A test is also provided which confirms functionality.
> 
> 2. Opaque sorcery data
> 
> Previously sorcery data was placed directly in the structure definition for objects. This had the consequence of requiring any modules using the structures to be recompiled if the sorcery structure changed. The change attached adds a generic sorcery object allocation function which allocates enough room for an internal opaque structure. This has the benefits that modules do not need to be recompiled if internal details change and it also allows additional data to be attached to sorcery objects and properly deallocated when the object is destroyed.
> 
> 
> Diffs
> -----
> 
>   /trunk/include/asterisk/sorcery.h 390335 
>   /trunk/main/sorcery.c 390335 
>   /trunk/res/res_sip/config_auth.c 390335 
>   /trunk/res/res_sip/config_domain_aliases.c 390335 
>   /trunk/res/res_sip/config_transport.c 390335 
>   /trunk/res/res_sip/location.c 390335 
>   /trunk/res/res_sip/sip_configuration.c 390335 
>   /trunk/tests/test_sorcery.c 390335 
> 
> Diff: https://reviewboard.asterisk.org/r/2585/diff/
> 
> 
> Testing
> -------
> 
> Ran unit tests, confirmed all happy. Used Gulp functionality and confirmed it is also happy.
> 
> 
> Thanks,
> 
> Joshua Colp
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20130614/dfbb5fec/attachment-0001.htm>