osmo-sip-connector[master]: sdp.c Send octet-align in fmtp

[Neels Hofmeyr]

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 <keith at rhizomatica.org>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Holger Freyther <holger at freyther.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Keith Whyte <keith at rhizomatica.org>
Gerrit-Reviewer: Max <msuraev at sysmocom.de>
Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr at sysmocom.de>
Gerrit-HasComments: Yes