[asterisk-dev] [Code Review] 2591: Add a _remove method for routes on a message_router.

[opticron]

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



/team/qwell/queue_events/include/asterisk/stasis_message_router.h
<https://reviewboard.asterisk.org/r/2591/#comment17293>

    This needs a \retval and some spacing would be nice.



/team/qwell/queue_events/main/stasis_message_router.c
<https://reviewboard.asterisk.org/r/2591/#comment17295>

    You should assert that OBJ_KEY is not set since it is not handled in both of these.



/team/qwell/queue_events/main/stasis_message_router.c
<https://reviewboard.asterisk.org/r/2591/#comment17294>

    This is unnecessary.



/team/qwell/queue_events/main/stasis_message_router.c
<https://reviewboard.asterisk.org/r/2591/#comment17296>

    Implementing OBJ_KEY would make half the code in this function go away since that's essentially what you're doing anyway.



/team/qwell/queue_events/main/stasis_message_router.c
<https://reviewboard.asterisk.org/r/2591/#comment17297>

    I suggest removing the internal locking from remove_route and find_route and adding scoped router locking here instead since every other call of find_route is surrounded by a lock of the router.


- opticron


On June 4, 2013, 11:20 a.m., Jason Parker wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2591/
> -----------------------------------------------------------
> 
> (Updated June 4, 2013, 11:20 a.m.)
> 
> 
> Review request for Asterisk Developers and David Lee.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> Requested on review 2586.
> 
> Convert router routes to ao2_containers and add a way of removing them.
> 
> 
> Diffs
> -----
> 
>   /team/qwell/queue_events/include/asterisk/stasis_message_router.h 390393 
>   /team/qwell/queue_events/main/stasis_message_router.c 390393 
> 
> Diff: https://reviewboard.asterisk.org/r/2591/diff/
> 
> 
> Testing
> -------
> 
> Stuff didn't crash when I unloaded my module that removes routes.  Messages still got routed through AMI.
> 
> 
> Thanks,
> 
> Jason Parker
> 
>

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