[Asterisk-code-review] res srtp: Enable AES-256 and AES-GCM. (asterisk[master])

[Alexander Traud]

Alexander Traud has posted comments on this change.

Change subject: res_srtp: Enable AES-256 and AES-GCM.
......................................................................


Patch Set 2:

(4 comments)

https://gerrit.asterisk.org/#/c/3183/2/channels/chan_sip.c
File channels/chan_sip.c:

PS2, Line 13236: 		if (ast_strlen_zero(orig_crypto)) {
               : 			return NULL;
               : 		}
> If 'orig_crypto' is empty do you want to return here or simply continue to 
Done


PS2, Line 13239: if (ast_asprintf(&a_crypto, "%sa=crypto:%s\r\n", copy, orig_crypto) == -1) {
               : 			return NULL;
               : 		}
> Potential memory leak of 'a_crypto' in this off nominal path.
Done


PS2, Line 13232: 	do {
               : 		char *copy = a_crypto;
               : 
               : 		orig_crypto = ast_sdp_srtp_get_attrib(tmp, dtls_enabled, default_taglen_32);
               : 		if (ast_strlen_zero(orig_crypto)) {
               : 			return NULL;
               : 		}
               : 		if (ast_asprintf(&a_crypto, "%sa=crypto:%s\r\n", copy, orig_crypto) == -1) {
               : 			return NULL;
               : 		}
               : 	} while ((tmp = AST_LIST_NEXT(tmp, sdp_srtp_list)));
> I think there is potential for a memory leak in this code. asprintf always 
Done


https://gerrit.asterisk.org/#/c/3183/2/main/sdp_srtp.c
File main/sdp_srtp.c:

PS2, Line 612: sizeof(attr) / sizeof(attr[0]);
> Should be able to use the ARRAY_LEN macro here found in utils.h
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I11326d80edd656524a51a19450e586c583aa0a0b
Gerrit-PatchSet: 2
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Alexander Traud <pabstraud at compuserve.com>
Gerrit-Reviewer: Alexander Traud <pabstraud at compuserve.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-HasComments: Yes