[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 2: Code-Review-1

(4 comments)

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

PS2, Line 13: * They cannot be merged with other bridges.
            : * They cannot be involved in transfers.
Delete these two restriction lines.

I think these restrictions should not be part of the cloak of invisibility.  They are handled by other bridge flags.  In fact when we get real early bridges they will need to be involved in blonde transfers.


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

PS2, Line 769: 	} else {
             : 		/* Invisible bridges cannot be merged or swapped to/from. In case those flags
             : 		 * are not already set, go ahead and do it now.
             : 		 */
             : 		ast_set_flag(&self->feature_flags, AST_BRIDGE_FLAG_MERGE_INHIBIT_FROM
             : 			| AST_BRIDGE_FLAG_MERGE_INHIBIT_TO
             : 			| AST_BRIDGE_FLAG_SWAP_INHIBIT_FROM
             : 			| AST_BRIDGE_FLAG_SWAP_INHIBIT_TO);
delete these lines

If these properties are needed then the bridge needs to be created with them.  They should not be forced.


PS2, Line 4336: 	if (bridge && (ast_test_flag(&bridge->feature_flags,
              : 			AST_BRIDGE_FLAG_MASQUERADE_ONLY | AST_BRIDGE_FLAG_INVISIBLE))) {
The added opening parentheses is in the wrong place.  It should be here -> (AST_BRIDGE_FLAG_MASQUERADE_ONLY |...


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

PS2, Line 235: 	if (ast_test_flag(&bridge->feature_flags, AST_BRIDGE_FLAG_INVISIBLE)) {
             : 		return NULL;
             : 	}
Put this test back into the ast_bridge_publish_enter() and ast_bridge_publish_leave() functions.  The test there will prevent needless allocation/free before the snapshot creation blocks the publish.


-- 
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: 2
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