[asterisk-dev] [Code Review] HD ConfBridge application

[jrose]

> On 2011-04-06 16:58:47, Russell Bryant wrote:
> > /trunk/apps/app_confbridge.c, line 1176
> > <https://reviewboard.asterisk.org/r/1147/diff/3/?file=15999#file15999line1176>
> >
> >     = { '\0', }; would be a bit more explicit, but I would just do = "";
> >     
> >     Also, what is MAX_PIN supposed to be?  I would read that as the maximum length of a pin, when in reality, the max length of a pin is MAX_PIN - 1 (because of the nul terminator).
> 
> jrose wrote:
>     Actually, MAX_PIN would still be the maximum length of the pin.  The null terminator is in the MAX_PINth + 1 position since the array starts at 0.  But you know that :P
>     
>     I'm not sure I'm working that properly, so I'll just say... if MAX_PIN is 4 and the array length is 8...
>     0 - char 1
>     1 - char 2
>     2 - char 3
>     3 - char 4
>     4 - '\0'
> 
> jrose wrote:
>     Oh wait, this is a declaration.  Disregard everything I just said.

Actually though, wouldn't that terminator be at the beginning of the line resulting in the possibility of the array having no terminator after it is set?  It might seem like bad form for a string, but the code might always be using comparison functions that take the length into account, which would mean that this could still be the maximum pin length.


- jrose


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


On 2011-03-28 15:47:43, David Vossel wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1147/
> -----------------------------------------------------------
> 
> (Updated 2011-03-28 15:47:43)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> The new ConfBridge application.  It's kind of a big deal.
> 
> 
> Diffs
> -----
> 
>   /trunk/configs/confbridge.conf.sample PRE-CREATION 
>   /trunk/include/asterisk/bridging.h 311748 
>   /trunk/include/asterisk/bridging_features.h 311748 
>   /trunk/include/asterisk/bridging_technology.h 311748 
>   /trunk/include/asterisk/channel.h 311748 
>   /trunk/include/asterisk/dsp.h 311748 
>   /trunk/main/bridging.c 311748 
>   /trunk/main/channel.c 311748 
>   /trunk/main/dsp.c 311748 
>   /trunk/apps/confbridge/include/confbridge.h PRE-CREATION 
>   /trunk/bridges/bridge_builtin_features.c 311748 
>   /trunk/bridges/bridge_softmix.c 311748 
>   /trunk/apps/confbridge/conf_config_parser.c PRE-CREATION 
>   /trunk/apps/app_confbridge.c 311748 
>   /trunk/CHANGES 311748 
>   /trunk/UPGRADE.txt 311748 
>   /trunk/apps/Makefile 311748 
>   /trunk/res/res_musiconhold.c 311748 
> 
> Diff: https://reviewboard.asterisk.org/r/1147/diff
> 
> 
> Testing
> -------
> 
> All confbridge.conf features have been tested.
> Load tested at sample rates ranging from 8-48khz.
> AMI actions/events tested
> CLI commands tested
> 
> 
> Thanks,
> 
> David
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20110420/797d9a17/attachment.htm>