Patch Set 6: Code-Review+1 (1 comment)
https://gerrit.osmocom.org/#/c/3735/6/src/sdp.c File src/sdp.c: Line 174: fmtp_str = talloc_asprintf(leg,"a=fmtp:%d octet-align=1 mode-set=4\r\n",other->payload_type); (rather have a space after each comma) The invocation in itself is correct, the question is more about memory management. fmtp_str is a local string that can safely be freed after below talloc_asprintf() is done. You're allocating it in the 'leg' context, so it will be freed once the 'leg' ctx is freed. So, how often will this be called before 'leg' is going to be freed? If we call this a thousand times, we would create a thousand such unused allocations and an explicit free before returning would make a real positive impact. Otherwise it doesn't matter much, though an explicit free would be more sanitary. If you free it, of course you won't be able to start out with a "" string constant, because we must not attempt to talloc_free(""). instead e.g. use fmtp_str = NULL; [...] talloc_asprintf(..., fmtp_str ? fmtp_str : "" ) BTW, the same applies to the returned value: does the caller free it? Does it matter? This is of course present before this patch and hopefully the author had it figured out. -- To view, visit https://gerrit.osmocom.org/3735 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I938758ac4ec55db9223e3da6c3c277e8fa670055 Gerrit-PatchSet: 6 Gerrit-Project: osmo-sip-connector Gerrit-Branch: master Gerrit-Owner: Keith Whyte <[email protected]> Gerrit-Reviewer: Harald Welte <[email protected]> Gerrit-Reviewer: Holger Freyther <[email protected]> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Keith Whyte <[email protected]> Gerrit-Reviewer: Max <[email protected]> Gerrit-Reviewer: Neels Hofmeyr <[email protected]> Gerrit-HasComments: Yes
