[Asterisk-code-review] feat: AudioSocket channel and application (...asterisk[master])

[Sean Bright]

Sean Bright has posted comments on this change. ( https://gerrit.asterisk.org/c/asterisk/+/11579 )

Change subject: feat: AudioSocket channel and application
......................................................................


Patch Set 5: Code-Review-1

(7 comments)

https://gerrit.asterisk.org/#/c/11579/5/channels/chan_audiosocket.c 
File channels/chan_audiosocket.c:

https://gerrit.asterisk.org/#/c/11579/5/channels/chan_audiosocket.c@170 
PS5, Line 170: 	instance = ast_calloc(1, sizeof(*instance));
Need to check that calloc() succeeded.


https://gerrit.asterisk.org/#/c/11579/5/channels/chan_audiosocket.c@177 
PS5, Line 177: 		struct ast_str *cap_buf = ast_str_alloca(AST_FORMAT_CAP_NAMES_LEN);
Need to check that ast_str_alloc() succeeded.


https://gerrit.asterisk.org/#/c/11579/5/res/res_audiosocket.c 
File res/res_audiosocket.c:

https://gerrit.asterisk.org/#/c/11579/5/res/res_audiosocket.c@74 
PS5, Line 74: 		if ((s = socket(addrs[i].ss.ss_family, SOCK_STREAM, IPPROTO_TCP)) < 0) {
            : 			ast_log(LOG_WARNING, "unable to create socket: %s\n", strerror(errno));
            : 			continue;
            : 		}
            : 
            : 		if (ast_fd_set_flags(s, O_NONBLOCK)) {
            : 			close(s);
            : 			continue;
            : 		}
If you use `ast_socket_nonblock()` instead of `socket()` you can remove the explicit call to `ast_fd_set_flags()`.


https://gerrit.asterisk.org/#/c/11579/5/res/res_audiosocket.c@168 
PS5, Line 168: 	//char idBuf[AST_UUID_STR_LEN+1];
Shouldn't use C++ comment syntax. There are several places in this file where this occurs, so make sure to take care of them all. This particular line should probably just be deleted altogether.


https://gerrit.asterisk.org/#/c/11579/5/res/res_audiosocket.c@181 
PS5, Line 181: 	buf = ast_malloc(3 + 16);
Is there a reason not to just stack allocate here? e.g. uint8_t buf[19]


https://gerrit.asterisk.org/#/c/11579/5/res/res_audiosocket.c@204 
PS5, Line 204: 	buf = ast_malloc(3 + f->datalen);
Need to check that malloc() succeeded.


https://gerrit.asterisk.org/#/c/11579/5/res/res_audiosocket.c@271 
PS5, Line 271: 	data = ast_malloc(len);
Need to check that malloc() succeeded.



-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/11579
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Change-Id: Ie866e6c4fa13178ec76f2a6971ad3590a3a588b5
Gerrit-Change-Number: 11579
Gerrit-PatchSet: 5
Gerrit-Owner: Seán C. McCord <ulexus at gmail.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: Sean Bright <sean.bright at gmail.com>
Gerrit-Reviewer: Seán C. McCord <ulexus at gmail.com>
Gerrit-Comment-Date: Thu, 18 Jul 2019 20:29:18 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20190718/5a661ab4/attachment.html>