[asterisk-dev] [Code Review] astobj2: Avoid using temporary objects + ao2_find() with OBJ_POINTER

[Terry Wilson]

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



/trunk/channels/chan_iax2.c
<https://reviewboard.asterisk.org/r/1184/#comment7278>

    I like the idea in general. Cutting out boilerplate code for the common case makes a lot of sense. I'm assuming code like the above would eventually be simplified to not handle the OBJ_POINTER case? It doesn't really simplify much to have to support both in the callback function.
    
    If that is the cae, I second the Ship It! if it actually works. :-)


- Terry


On 2011-04-18 12:34:34, Russell Bryant wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1184/
> -----------------------------------------------------------
> 
> (Updated 2011-04-18 12:34:34)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> There is a fairly common pattern making its way through the code base where we put a temporary object on the stack so we can call ao2_find() with OBJ_POINTER.  The purpose is so that it can be passed into the object hash function.  However, this really seems like a hack and potentially error prone.  This patch is a first stab at approach to avoid having to do that.
> 
> It adds a new flag, OBJ_KEY, which can be used instead of OBJ_POINTER in these situations.  Then, the hash function can know whether it was given an object or some custom data to hash.
> 
> The patch also changes some uses of ao2_find() for iax2_user and iax2_peer objects to reflect how OBJ_KEY would be used.
> 
> 
> Diffs
> -----
> 
>   /trunk/channels/chan_iax2.c 314080 
>   /trunk/include/asterisk/astobj2.h 314080 
>   /trunk/main/astobj2.c 314080 
> 
> Diff: https://reviewboard.asterisk.org/r/1184/diff
> 
> 
> Testing
> -------
> 
> It compiles.  I'm just throwing out the idea right now.  If people seem to think it makes sense, I will certainly try calls before committing anything.
> 
> 
> Thanks,
> 
> Russell
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20110524/852bf38e/attachment.htm>