[asterisk-dev] [Code Review] chan_dahdi: create and destroy channels at run-time

[rmudgett]

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


Enough for now since this is an incomplete patch.


/trunk/channels/chan_dahdi.c
<https://reviewboard.asterisk.org/r/1598/#comment9105>

    Doxygen description for new members.



/trunk/channels/chan_dahdi.c
<https://reviewboard.asterisk.org/r/1598/#comment9104>

    Update comment and doxegnify.



/trunk/channels/chan_dahdi.c
<https://reviewboard.asterisk.org/r/1598/#comment9111>

    I think you should not output these verbose messages for just any caller.  The do_monitor() caller should not have verbose message output.
    
    Possibly you could pass in the fd if output is needed and pass in a -1 if not needed.



/trunk/channels/chan_dahdi.c
<https://reviewboard.asterisk.org/r/1598/#comment9112>

    Curly braces.



/trunk/channels/chan_dahdi.c
<https://reviewboard.asterisk.org/r/1598/#comment9115>

    You did not need to move this prototype.



/trunk/channels/chan_dahdi.c
<https://reviewboard.asterisk.org/r/1598/#comment9106>

    Doxygen function description.



/trunk/channels/chan_dahdi.c
<https://reviewboard.asterisk.org/r/1598/#comment9118>

    Adding channels to an existing ISDN span is going to have subtle problems.  This check is not adequate to prevent it.
    
    Also adding SS7 channels to existing linksets is likely to have other problems due to the weird way SS7 configures channels.



/trunk/channels/chan_dahdi.c
<https://reviewboard.asterisk.org/r/1598/#comment9107>

    Curly braces.



/trunk/channels/chan_dahdi.c
<https://reviewboard.asterisk.org/r/1598/#comment9116>

    This must be done for all exit points in the function to not have a memory leak.  This is why setup_dahdi_int() was extracted from setup_dahdi().



/trunk/channels/chan_dahdi.c
<https://reviewboard.asterisk.org/r/1598/#comment9117>

    Comma spacing.



/trunk/channels/chan_dahdi.c
<https://reviewboard.asterisk.org/r/1598/#comment9102>

    if (sscanf() == 1)



/trunk/channels/chan_dahdi.c
<https://reviewboard.asterisk.org/r/1598/#comment9103>

    Might as well fix the spacing with the parentheses.



/trunk/channels/chan_dahdi.c
<https://reviewboard.asterisk.org/r/1598/#comment9110>

    if (sscanf() == 1)



/trunk/channels/chan_dahdi.c
<https://reviewboard.asterisk.org/r/1598/#comment9109>

    Spacing with parentheses.



/trunk/channels/chan_dahdi.c
<https://reviewboard.asterisk.org/r/1598/#comment9113>

    Odd line breaks.
    Curly braces needed.



/trunk/channels/chan_dahdi.c
<https://reviewboard.asterisk.org/r/1598/#comment9114>

    Memory leak in conf when goto path taken.
    Need to call ast_cc_config_params_destroy().
    
    Why not just check for the pseudo channel before entering this if block?


- rmudgett


On Nov. 24, 2011, 12:19 p.m., Tzafrir Cohen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1598/
> -----------------------------------------------------------
> 
> (Updated Nov. 24, 2011, 12:19 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> This code adds chan_dahdi the command 'dahdi create channels <range>' (where <range> is a single <n>-<m>) and updates 'dahdi destroy channel' with a similar 'dahdi destroy channels'.
> 
> 'dahdi create channels' acts as a filter for process_dahdi() and such in DAHDI: it re-parses the whole configuration, but only attempts to create channels in the range.
> 
> Right not this patch disables the initial probing at module load time (workaround: run 'dahdi create channels 1-10000' at startup through cli.conf). This is for debugging, mainly and should be removed later on.
> 
> This changeg is intended to provide a hook for a script running from udev once a span has been assigned ("registered") / unassigned ("unregistered") for its channels. The udev hook configures the span's channels with dahdi_cfg -S, and can then ask Asterisk to create ethe channels.
> 
> See: https://gitorious.org/~tzafrir/asterisk-tools/tzafrirs-dahdi-tools/commits/pinned-spans
> 
> This means that a separate DAHDI init script (running before the Asterisk one) would no longer be required. There is no longer a need to run a single command after all DAHDI devices are up.
> 
> 
> Diffs
> -----
> 
>   /trunk/channels/chan_dahdi.c 346290 
> 
> Diff: https://reviewboard.asterisk.org/r/1598/diff
> 
> 
> Testing
> -------
> 
> Works well with analog devices. Seems to work with ISDN ones as well. Not fully complete, though.
> 
> 
> Thanks,
> 
> Tzafrir
> 
>

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