osmo-mgw[master]: sdp: refactoring sdp parser/generator

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/.

Neels Hofmeyr gerrit-no-reply at lists.osmocom.org
Sun Oct 22 19:58:20 UTC 2017


Patch Set 13: Code-Review-1

(10 comments)

https://gerrit.osmocom.org/#/c/4006/13/src/libosmo-mgcp/mgcp_sdp.c
File src/libosmo-mgcp/mgcp_sdp.c:

Line 41: /*! set codec configuration depending on payload type and codec name.
Nice, ends in a full stop, now all you need is to start with a capital letter :)


Line 42:  *  \endp[in] ctx talloc context
\param


Line 147: 			  int payload, char *audio_name)
Let's constify all pointer args that aren't modified.


Line 195:  *  \endp[in] endp trunk endpoint
that should be '\param', not '\endp'


Line 197:  *  \endp[in] caller provided memory to store the parsing results
technically, that's an [out], then. Plus, there is no 'caller' arg in the function signature.


Line 198:  *  \returns 0 on success, -1 on failure */
it would be good to always include a '.' at the end of each \param, each \returns and each sentence in general. In doxygen, line breaks will not be visible, adding '.' clarifies things.


Line 200: 			struct mgcp_parse_data *p)
Let's constify all pointer args that aren't modified.


Line 328: /*! generate SDP response string.
I wonder if this should use msgb_printf() that we added recently instead of returning to a fixed string buffer.


Line 329:  *  \endp[in] endp trunk endpoint
\param


Line 335: int mgcp_write_response_sdp(struct mgcp_endpoint *endp,
> not sure if endp and/or conn could be 'const' (read-only), but this is not 
since it's public API the good move would be to const the args now.


-- 
To view, visit https://gerrit.osmocom.org/4006
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I9f88c93872ff913bc211f560b26901267f577324
Gerrit-PatchSet: 13
Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Owner: Neels Hofmeyr <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: dexter <pmaier at sysmocom.de>
Gerrit-HasComments: Yes



More information about the gerrit-log mailing list