[asterisk-dev] [Code Review] 2998: Push Security Events over AMI

[jrose]

> On Nov. 6, 2013, 7:32 p.m., Mark Michelson wrote:
> > /branches/12/include/asterisk/security_events.h, line 96
> > <https://reviewboard.asterisk.org/r/2998/diff/2/?file=48001#file48001line96>
> >
> >     The name of this function is a bit on the awkward side. Why is it called this?
> 
> jrose wrote:
>     It's called that because it's a secondary initialization function that must take place after the initialization of manager (more specifically, once its stasis topic has been created). I'm all ears on a better name for it... or just making the decision to delay the initialization of the security events framework until after manager is initialized. It probably doesn't matter since either way, nothing is logging the security events until either the forwarder is initialized or else a resource that subscribes to the security event stasis messages is loaded... and since this occurs right after manager, it's probably going to be the forwarder.
> 
> Matt Jordan wrote:
>     I was thinking about this some and wanted to chime in before I popped out for the weekend.
>     
>     If we can, I'd recommend that we find another way to initialize the forwarder and/or another place for it to live. This only has to be created - it never gets actually *used* by security events. For all intents and purposes, this could be created in Manager, in which case you'd no longer need this at all.
> 
> Matt Jordan wrote:
>     "No longer need this at all" => the function itself. You still need the forwarder.

Alrighty, I moved the forwarder to manager.c and initialize and destroy it alongside the rtp topic forwarder that's already in there.


- jrose


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


On Nov. 6, 2013, 5:54 p.m., jrose wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2998/
> -----------------------------------------------------------
> 
> (Updated Nov. 6, 2013, 5:54 p.m.)
> 
> 
> Review request for Asterisk Developers, kmoore, Matt Jordan, and Mark Michelson.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> Adds a to AMI handler for security event messages. I'm not 100% sure if it's proper to initialize the message forwarder in a secondary initialization step like I am, but I wasn't sure if I should move the initialization of the entire security event framework to after manager initializes instead.
> 
> 
> Diffs
> -----
> 
>   /branches/12/main/security_events.c 402500 
>   /branches/12/main/manager.c 402500 
>   /branches/12/main/asterisk.c 402500 
>   /branches/12/include/asterisk/security_events.h 402500 
>   /branches/12/include/asterisk/manager.h 402500 
> 
> Diff: https://reviewboard.asterisk.org/r/2998/diff/
> 
> 
> Testing
> -------
> 
> Tested that security events get logged over AMI.  Tested that obvious memory leaks against strings and the security event system aren't occurring with MALLOC_DEBUG.
> 
> 
> Thanks,
> 
> jrose
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20131107/fa1e84c3/attachment.html>