[Asterisk-code-review] chan sip: Enable WebSocket support by default. (asterisk[master])

[Kevin Harwell]

Kevin Harwell has posted comments on this change.

Change subject: chan_sip: Enable WebSocket support by default.
......................................................................


Patch Set 1:

The only thing I was not quite sure about this fix is how the associated start/stop code is handled.

Before intializing the option the following occurs:

int websocket_was_enabled = sip_cfg.websocket_enabled;

Then later after setting the options:

if (reason != CHANNEL_MODULE_LOAD && websocket_was_enabled != sip_cfg.websocket_enabled) {
    if (sip_cfg.websocket_enabled) {
	ast_websocket_add_protocol("sip", sip_websocket_callback);
    } else {
	ast_websocket_remove_protocol("sip", sip_websocket_callback);
    }
}

The first pass through should be when the module is loading, so this check is bypassed and the websocket is not started. However, the configuration option is set is set to TRUE now (via the default). When it enters the function later, not during load, then websocket_was_enabled gets set to TRUE and the websocket_enabled option is TRUE, so it will fall into the remove_protocol section correct?

Just seems like the websocket will never be started.

However I could totally be misreading this part of the code, but thought I would bring it up just in case.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Icb02bbcad47b11a795c14ce20a9bf29649a54423
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-HasComments: No