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

[Mark Michelson]

Mark Michelson has posted comments on this change.

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


Patch Set 5:

(3 comments)

My only comments are extremely minor, so I'm not even bothering with putting a -1 on.

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

Line 41: /*! Initial size of the a message queue. Remember to keep it a prime number! */
The directive to keep it a prime number here doesn't really make sense. The other constants in this file are the sizes of hash tables, and the prime number factors into the decreased probability of generating hash collisions. With the message queue, it doesn't really matter if it's prime.


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

Line 785: 		size = sizeof(*session) + strlen(session_id) + 1;
A minor thing, but I suggest using either sizeof(session_id) (preferred) or AST_UUID_STR_LEN (not as preferred) here instead of strlen. You're basically able to use a compile time constant instead of calculating at runtime.


Line 795: 		strncpy(session->session_id, session_id, size - sizeof(*session));
The subtraction isn't necessary here since you can either use sizeof(session_id) or AST_UUID_STR_LEN.


-- 
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: 5
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: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Matt Jordan <mjordan at digium.com>
Gerrit-HasComments: Yes