[asterisk-dev] [Code Review] 726: handle DAHDI_EVENT_REMOVED on a D-Channel

[rmudgett]

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

Ship it!


Other than changing the usage of strtol() to use sscanf(), the rest are just formatting issues.  Just correct these issues and commit.


/trunk/channels/chan_dahdi.c
<https://reviewboard.asterisk.org/r/726/#comment18034>

    Put curly on the following line.



/trunk/channels/chan_dahdi.c
<https://reviewboard.asterisk.org/r/726/#comment18033>

    Curly braces are needed here.



/trunk/channels/chan_dahdi.c
<https://reviewboard.asterisk.org/r/726/#comment18036>

    Use sscanf() instead of strtol().
    
    if (sscanf(channel, "%30d", &chan_num) != 1) {
    	/* Not numeric string. */
    	return NULL;
    }
    



/trunk/channels/chan_dahdi.c
<https://reviewboard.asterisk.org/r/726/#comment18035>

    Spacing issues around operators.
    Spaces around = and -.  Also the variable declarations should have a space after the ','.  I would prefer that they were declared one per line, but that's my preference.



/trunk/channels/chan_dahdi.c
<https://reviewboard.asterisk.org/r/726/#comment18037>

    Parentheses are not necessary for the &().


- rmudgett


On June 16, 2013, 5:51 p.m., Tzafrir Cohen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/726/
> -----------------------------------------------------------
> 
> (Updated June 16, 2013, 5:51 p.m.)
> 
> 
> Review request for Asterisk Developers and rmudgett.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> When a DAHDI device is removed at run-time it sends the event DAHDI_EVENT_REMOVED on each channel. This is intended to signal the userspace program to close the respective file handle, as the driver of the device will need all of them closed to properly clean-up.
> 
> This event has long since been handled in chan_dahdi (chan_zap at the time). However the event that is sent on a D-Channel of a "PRI" (ISDN) span simply gets ignored.
> 
> This code adds handling for closing the file descriptor (and shutting down the span, while we're at it).
> 
> It also adds a CLI command 'pri destroy span <N>' to destroy the span and its DAHDI channels.
> 
> This code seems to be rather DAHDI-generic and thus I opted not to change anything in sig_pri.c .
> 
> I use a simplified version of the container_of() macro from the Linux Kernel, which basically returns a pointer to a struct that contains a different struct to which we already have a pointer. Thus I assume this will always be run with on a struct sig_pri_pri that is contained ina s struct dahdi_pri .
> 
> I'm not sure what proper locking needs doing.
> 
> 
> Diffs
> -----
> 
>   /trunk/channels/chan_dahdi.c 391921 
> 
> Diff: https://reviewboard.asterisk.org/r/726/diff/
> 
> 
> Testing
> -------
> 
> Disconnecting either using the CLI command or with device removal for some E1 and some BRI devices.
> 
> 
> Thanks,
> 
> Tzafrir Cohen
> 
>

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