[asterisk-dev] [Code Review]: Preserve DTMF length in main/features.c

[Russell Bryant]

I think establishing a policy of who is allowed to give a "ship it" is a
very good idea.  I do not think that being a Digium employee is the right
criteria.  There are people outside of Digium that should be able to ACK a
patch.  Also, a new hire at Digium may not be appropriate to give an ACK,
either.

I would propose a 2-tier committers system:

 - A committer (someone with commit access)
 - A reviewer (a committer that can sign off on non-trivial changes on
reviewboard)

Then you have to answer how one can become a committer or a reviewer.  I'll
kick off the topic with proposing:

 - A contributer can become a committer if backed by a reviewer.
 - A committer can become a reviewer if backed by 2 other reviewers.

Obviously everyone who already has commit access would be the committers.
The initial reviewer pool would have to be seeded by project leadership
(Kevin, with input from others, I suppose).

--
Russell Bryant


On Mon, Oct 24, 2011 at 5:28 PM, jrose <reviewboard at asterisk.org> wrote:

>    This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1463/
>
> On October 24th, 2011, 4:24 p.m., *jrose* wrote:
>
>   /trunk/main/features.c<https://reviewboard.asterisk.org/r/1463/diff/1/?file=20901#file20901line3911> (Diff
> revision 1)
>
> int ast_bridge_call(struct ast_channel *chan, struct ast_channel *peer, struct ast_bridge_config *config)
>
>   3911
>
> 						ast_dtmf_stream(chan, peer, peer_featurecode, 0, 0);
>
>  3911
>
> 						ast_dtmf_stream(chan, peer, peer_featurecode, 0, f->len);
>
>    3912
>
> 						memset(peer_featurecode, 0, sizeof(peer_featurecode));
>
>  3912
>
> 						memset(peer_featurecode, 0, sizeof(peer_featurecode));
>
>   3913
>
> 					}
>
> 3913
>
> 					}
>
>  3914
>
> 					if (!ast_strlen_zero(chan_featurecode)) {
>
>  3914
>
> 					if (!ast_strlen_zero(chan_featurecode)) {
>
>    3915
>
> 						ast_dtmf_stream(peer, chan, chan_featurecode, 0, 0);
>
>  3915
>
> 						ast_dtmf_stream(peer, chan, chan_featurecode, 0, f->len);
>
>   f->len is not good here.  Frame can very easily be null within this block of code, and if it is, trying to access f->len will cause a segfault... and in fact, messing about with google voice with the interns today exposed this very problem.
>
> Generally speaking you need to wait for someone internal to give a ship-it before committing code if you have commit access.  I don't actually know whether that status of people on reviewboard is actually viewable anywhere, so I guess for now the only real answer to that is that if you don't know, you need to ask on IRC.  I'm going to go ahead and fix these items since it's a pretty easy change, but just keep that in mind int he future.
>
>  s/someone internal/Digium Developer
>
> *in the future
>
>
> - jrose
>
> On September 27th, 2011, 8:53 a.m., Olle E Johansson wrote:
>   Review request for Asterisk Developers.
> By Olle E Johansson.
>
> *Updated Sept. 27, 2011, 8:53 a.m.*
> Description
>
> When DTMF goes through features (because we're listening for some feature codes) the length is disregarded. This patch makes sure that the DTMF length doesn't disappear in a few cases.
>
>   Testing
>
> This is part of a larger branch that fixes a lot of DTMF issues.
>
>   Diffs
>
>    - /trunk/main/features.c (338029)
>
> View Diff <https://reviewboard.asterisk.org/r/1463/diff/>
>
> --
> _____________________________________________________________________
> -- Bandwidth and Colocation Provided by http://www.api-digital.com --
>
> asterisk-dev mailing list
> To UNSUBSCRIBE or update options visit:
>   http://lists.digium.com/mailman/listinfo/asterisk-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20111024/85f0c80b/attachment.htm>