Change in osmo-msc[master]: add sdp_msg API: SDP parsing/composition

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.

pespin gerrit-no-reply at lists.osmocom.org
Mon Nov 4 10:38:27 UTC 2019


pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-msc/+/15947 )

Change subject: add sdp_msg API: SDP parsing/composition
......................................................................


Patch Set 1:

(4 comments)

> Patch Set 1:
> 
> doesn't it much rather make sense to have at least a helper library containing those things that you are copy+pasting here?  something like libosmo-sdp-utils, which then can be used by any of the actual parser codebases?  Copy+Pasting of such code really hurts me :/

Agree.

https://gerrit.osmocom.org/c/osmo-msc/+/15947/1/include/osmocom/msc/debug.h 
File include/osmocom/msc/debug.h:

https://gerrit.osmocom.org/c/osmo-msc/+/15947/1/include/osmocom/msc/debug.h@38 
PS1, Line 38: 	size_t len = 64; \
Makes much more sense having len as a param to avoid almost-sure talloc_realloc on some functions.


https://gerrit.osmocom.org/c/osmo-msc/+/15947/1/include/osmocom/msc/debug.h@40 
PS1, Line 40: 	char *str = (char*)talloc_named_const(ctx, len, #FUNC_BUF "_c()"); \
where does this "ctx" var come from?


https://gerrit.osmocom.org/c/osmo-msc/+/15947/1/include/osmocom/msc/debug.h@63 
PS1, Line 63: #define NAME_IMPL(FUNC_C, FUNC_C_ARGS...) \
So you return a talloc'ed pointer on success and a static string pointer on error? That will break when you attempt to free it. I'd drop this function completely, null is your friend!


https://gerrit.osmocom.org/c/osmo-msc/+/15947/1/src/libmsc/sdp_msg.c 
File src/libmsc/sdp_msg.c:

https://gerrit.osmocom.org/c/osmo-msc/+/15947/1/src/libmsc/sdp_msg.c@105 
PS1, Line 105: 				   codec->fmtp[0] ? codec->fmtp : NULL);
shouldn't you compare against '\0' here?  codec->fmtp[0] != '\0' ? ...



-- 
To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/15947
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Change-Id: If3ce23cd5bab15e2ab4c52ef3e4c75979dffe931
Gerrit-Change-Number: 15947
Gerrit-PatchSet: 1
Gerrit-Owner: neels <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: laforge <laforge at osmocom.org>
Gerrit-CC: pespin <pespin at sysmocom.de>
Gerrit-Comment-Date: Mon, 04 Nov 2019 10:38:27 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20191104/26efc194/attachment.htm>


More information about the gerrit-log mailing list