osmo-mgw[master]: libosmo-mgcp: Connection Identifiers are allocated by MGW, n...

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

Harald Welte gerrit-no-reply at lists.osmocom.org
Thu Nov 23 08:15:12 UTC 2017


Patch Set 2:

(3 comments)

https://gerrit.osmocom.org/#/c/4905/2/src/libosmo-mgcp-client/mgcp_client.c
File src/libosmo-mgcp-client/mgcp_client.c:

Line 270: 	strncpy(r->head.conn_id, line + 3, sizeof(r->head.conn_id));
osmo_strlcpy will simplify those two lines


Line 292: 		*data_end = '\0';
it's not really that nice to modify input data, even though you restore it below.  Not critical to change, but in general definitely something we'd want to avoid.  Better probably to have a for_each_non_empty_line_until_empty_line() or the like, and then even mark the input argument to the function as "const"


https://gerrit.osmocom.org/#/c/4905/2/src/libosmo-mgcp/mgcp_conn.c
File src/libosmo-mgcp/mgcp_conn.c:

Line 33: static int mgcp_alloc_id(struct mgcp_endpoint *endp, char *id)
it *can* be up to 32 characters, but we could actually use much shorter ones.  I would have gone simply for an uint32_t.  Not critical.

Even though we don't have doxygen comments here, it would be good to state that "char *id" has to be caller-allocated and it is only the identifier that is allocated, not the memory in which the identifier is stored.


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

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



More information about the gerrit-log mailing list