[Asterisk-code-review] ARI: Channels added to Stasis application during WebSocket c... (asterisk[master])

[Ashley Sanders]

Ashley Sanders has posted comments on this change.

Change subject: ARI: Channels added to Stasis application during WebSocket creation ...
......................................................................


Patch Set 3: Code-Review-1

(6 comments)

https://gerrit.asterisk.org/#/c/993/3/res/ari/resource_events.c
File res/ari/resource_events.c:

Line 38: #define APPS_NUM_BUCKETS 11
> Why the change from 7 to 11?
That may be an artifact from a refactoring attempt gone awry. Since I honestly don't remember why I changed it, I will revert it back to 7.


Line 49: 	struct ao2_container *message_queue;           /*!< Container for holding delayed
> Doing this as an ao2 container with buckets doesn't make this a queue, it t
Ahhhhh, you're right. How. Did. I. Miss. This. Pivotal. Point... Thank you :)


Line 96: 	if (!session) {
> How can we not have a handle to the session?
Registering a stasis app is not localized to this object; it is an API function that can be invoked from anyone. Therefore, it would be possible that whomever registered a stasis app did so without providing a reference to the thing we need to process the event.


Line 108: 	} else if (!session->ws_session) {
> Is there a race condition here between ws_session being on the session and 
Yes. Good catch.


https://gerrit.asterisk.org/#/c/993/3/res/res_http_websocket.c
File res/res_http_websocket.c:

Line 90: 	char session_id[];                /*!< The identifier for the websocket session */
> UUIDs have a fixed size, you've used it in the variable below and might as 
Ah, yes. I am pretty sure that I declared this before I discovered the UUID size restriction. Thanks.


Line 689: 	char session_id[AST_UUID_STR_LEN];
> If you make the session_id a fixed size in the session you don't need this,
Got it. Thanks.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iafef7b85a2e0bf78c114db4c87ffc3d16d671a17
Gerrit-PatchSet: 3
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Ashley Sanders <asanders at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Ashley Sanders <asanders at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Matt Jordan <mjordan at digium.com>
Gerrit-HasComments: Yes