[Asterisk-code-review] Bridging: introduce "invisible" bridges. (asterisk[master])

[Richard Mudgett]

Richard Mudgett has posted comments on this change.

Change subject: Bridging: introduce "invisible" bridges.
......................................................................


Patch Set 1: Code-Review-1

(5 comments)

https://gerrit.asterisk.org/#/c/2789/1//COMMIT_MSG
Commit Message:

Line 13: * They cannot be merged with other bridges.
This functionality is already available with the AST_BRIDGE_FLAG_MERGE_INHIBIT_FROM and AST_BRIDGE_FLAG_MERGE_INHIBIT_TO flags.

Invisible bridges should also set the AST_BRIDGE_FLAG_SWAP_INHIBIT_FROM and AST_BRIDGE_FLAG_SWAP_INHIBIT_TO flags to prevent local channel optimizations.


https://gerrit.asterisk.org/#/c/2789/1/main/bridge.c
File main/bridge.c:

PS1, Line 2316: 	if (ast_test_flag(&dst_bridge->feature_flags, AST_BRIDGE_FLAG_INVISIBLE) || ast_test_flag(&src_bridge->feature_flags, AST_BRIDGE_FLAG_INVISIBLE)) {
              : 		ast_bridge_unlock(src_bridge);
              : 		ast_bridge_unlock(dst_bridge);
              : 		return -1;
              : 	}
This check is not needed if the invisible bridge is also created with the AST_BRIDGE_FLAG_MERGE_INHIBIT_FROM and AST_BRIDGE_FLAG_MERGE_INHIBIT_TO flags.


Line 4334: 		&& (ast_test_flag(&bridge->feature_flags, AST_BRIDGE_FLAG_MASQUERADE_ONLY | AST_BRIDGE_FLAG_INVISIBLE))) {
The opening parentheses was added in the wrong place.

Break long line.  It is best to break it before the masquerade only flag.


https://gerrit.asterisk.org/#/c/2789/1/main/stasis_bridges.c
File main/stasis_bridges.c:

PS1, Line 279: 	if (ast_test_flag(&bridge->feature_flags, AST_BRIDGE_FLAG_INVISIBLE)) {
             : 		return;
             : 	}
             : 
             : 	snapshot = ast_bridge_snapshot_create(bridge);
You can block many stasis messages by preventing snapshots of invisible bridges.  They should be camera shy.  You get the added benefit of preventing them from showing up in the CLI "bridge show all" and AMI BridgeList output because there are no snapshots.

You wouldn't need the check here.


PS1, Line 303: 	if (ast_test_flag(&bridge->feature_flags, AST_BRIDGE_FLAG_INVISIBLE)) {
             : 		return;
             : 	}
This check is redundant.  The function is only called by the ast_bridge_publish_enter() and ast_bridge_publish_leave() functions which have already checked the flag.


-- 
To view, visit https://gerrit.asterisk.org/2789
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I804a209d3181d7c54e3d61a60eb462e7ce0e3670
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-HasComments: Yes