osmo-mgw[master]: Initially implement the new osmo-mgw and libosmo-mgcp

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 1 23:23:07 UTC 2017


Patch Set 7:

(17 comments)

I did not take a close look at function, but so far spotted a number of cosmetics... it's also a lot, can't read all of it now.

https://gerrit.osmocom.org/#/c/4003/7/include/osmocom/mgcp/mgcp.h
File include/osmocom/mgcp/mgcp.h:

Line 36: #define RTP_PORT_DEFAULT_RANGE_START 16002
is 16002 a backwards compat consideration? rather start at 16000?


https://gerrit.osmocom.org/#/c/4003/7/include/osmocom/mgcp/mgcp_internal.h
File include/osmocom/mgcp/mgcp_internal.h:

Line 324: /*! \brief get the ip-address where the mgw application is bound on
let's not introduce more \brief comments. Firstly, we don't have doxygen here yet, secondly, in libosmocore we use AUTOBRIEF now. Hence just end the first sentence in a dot (as always!) and any future doxygen will use that as \brief implicitly.
see
https://osmocom.org/projects/cellular-infrastructure/wiki/Guidelines_for_API_documentation


https://gerrit.osmocom.org/#/c/4003/7/include/osmocom/mgcp/mgcp_msg.h
File include/osmocom/mgcp/mgcp_msg.h:

Line 4: /*
I guess we should add a sysmocom copyright including 2017 in it


https://gerrit.osmocom.org/#/c/4003/7/include/osmocom/mgcp/mgcp_stat.h
File include/osmocom/mgcp/mgcp_stat.h:

Line 4: /*
2017?


https://gerrit.osmocom.org/#/c/4003/7/include/osmocom/mgcp/osmux.h
File include/osmocom/mgcp/osmux.h:

Line 2: #define _OPENBSC_OSMUX_H_
(at some point we want to replace this with

  #pragma once

and drop the OPENBSC name)


https://gerrit.osmocom.org/#/c/4003/7/src/libosmo-mgcp/mgcp_conn.c
File src/libosmo-mgcp/mgcp_conn.c:

Line 73: /*! \brief allocate a new connection list entry
(no \brief as before, also below)


https://gerrit.osmocom.org/#/c/4003/7/src/libosmo-mgcp/mgcp_ep.c
File src/libosmo-mgcp/mgcp_ep.c:

Line 27: /* Endpoint typeset definistion */
definistion


https://gerrit.osmocom.org/#/c/4003/7/src/libosmo-mgcp/mgcp_msg.c
File src/libosmo-mgcp/mgcp_msg.c:

Line 4: /*
2017?


Line 71: /*! \brief Parse connection mode.
brief


https://gerrit.osmocom.org/#/c/4003/7/src/libosmo-mgcp/mgcp_network.c
File src/libosmo-mgcp/mgcp_network.c:

Line 55:  * 1/unit seconds. */
(I know this just changes the comment style; noticing that I don't fully get what 1/unit means, maybe an example would be good)


Line 76: /*! \brief send udp packet.
brief


Line 89: 	     len, inet_ntoa(*addr), ntohs(port));
printing int len as %u, probably causes compiler to warn


Line 190: 	    (int16_t) (seq - sstate->last_seq);
is this lindent again? let's rather not do this whitespace change.


Line 220: 	    ts_aligment_error(sstate, state->packet_duration, timestamp);
aligment? interestingly, the old function also was misspelled. should be "alignment"


Line 232: 		     (int32_t) (timestamp - sstate->last_timestamp),
lindent


Line 304: 					    state->timestamp_offset);
also unnecessary ws changes


Line 321: 		    0);
whitespace changes


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie008599136c7ed8a0dfbb0cf803188975a499fc5
Gerrit-PatchSet: 7
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: Holger Freyther <holger at freyther.de>
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