[asterisk-dev] [Code Review] Add a name to a sig_pri span

[rmudgett at digium.com]

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


I like the idea of a sig_pri_span.name[] member in the struct.  However, I see it as a replacement for the sig_pri_span.span member.  Thus, wherever span is used would be replaced with name instead.  For chan_dahdi, the name should be simply an ASCII number string to be compatible with the few places where span is used for something other than log messages.

If your intention was to just change the annoying diagnostic message, this is overkill.


/trunk/channels/sig_pri.h
<https://reviewboard.asterisk.org/r/800/#comment5450>

    How about using AST_CHANNEL_NAME here instead of 20.



/trunk/channels/sig_pri.c
<https://reviewboard.asterisk.org/r/800/#comment5446>

    This is going to be constant for all D channels in the span.  At the minimum it should be moved outside the for loop.
    
    The same can be said about the "Overlap Recv" line a couple lines below even though you did not add this line.


- rmudgett


On 2010-07-25 13:14:29, Tzafrir Cohen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/800/
> -----------------------------------------------------------
> 
> (Updated 2010-07-25 13:14:29)
> 
> 
> Review request for Asterisk Developers and rmudgett.
> 
> 
> Summary
> -------
> 
> sig_pri of chan_dahdi gives annoying messages such as:
> 
> [Jul 25 20:58:14] WARNING[2289]: sig_pri.c:985 pri_find_dchan: No D-channels available.  Using Primary channel as D-channel anyway.
> [Jul 25 20:58:14] WARNING[2290]: sig_pri.c:985 pri_find_dchan: No D-channels available.  Using Primary channel as D-channel anyway.
> [Jul 25 20:58:14] WARNING[2287]: sig_pri.c:985 pri_find_dchan: No D-channels available.  Using Primary channel as D-channel anyway.
> [Jul 25 20:58:14] WARNING[2288]: sig_pri.c:985 pri_find_dchan: No D-channels available.  Using Primary channel as D-channel anyway.
> 
> This change is intended to transform them to equally annoying and yet slightly more meaningful messages:
>  
> [Jul 25 20:58:14] WARNING[2289]: sig_pri.c:985 pri_find_dchan: Span DAHDI/3: No D-channels available.  Using Primary channel as D-channel anyway.
> [Jul 25 20:58:14] WARNING[2290]: sig_pri.c:985 pri_find_dchan: Span DAHDI/4: No D-channels available.  Using Primary channel as D-channel anyway.
> [Jul 25 20:58:14] WARNING[2287]: sig_pri.c:985 pri_find_dchan: Span DAHDI/1: No D-channels available.  Using Primary channel as D-channel anyway.
> [Jul 25 20:58:14] WARNING[2288]: sig_pri.c:985 pri_find_dchan: Span DAHDI/2: No D-channels available.  Using Primary channel as D-channel anyway.
> 
> As for the original issue, see https://issues.asterisk.org/view.php?id=17270 .
> 
> This fix adds a field called 'name' to the struct sig_pri_span, and uses it when displaying span information, and also in that specific message as a demonstration.
> 
> Note that the code in chan_dahdi.c:mkintf() at that point could use some indentation-level reduction as well at the same spot.
> 
> 
> Diffs
> -----
> 
>   /trunk/channels/chan_dahdi.c 279389 
>   /trunk/channels/sig_pri.h 279389 
>   /trunk/channels/sig_pri.c 279389 
> 
> Diff: https://reviewboard.asterisk.org/r/800/diff
> 
> 
> Testing
> -------
> 
> Lightly tested loading, unloading and such.
> 
> 
> Thanks,
> 
> Tzafrir
> 
>