Patch Set 1: Code-Review-1 (6 comments)
Sory. Correct in principle, but missing procedural requirements (TODO-RELEASE) and misleading wording :/ https://gerrit.osmocom.org/#/c/4317/1/include/osmocom/gprs/gprs_ns.h File include/osmocom/gprs/gprs_ns.h: Line 92: uint16_t remote_port; This breaks ABI compatibility and deserves a TODO-RELEASE entry to remind us to change LIBVERSION at the next release https://gerrit.osmocom.org/#/c/4317/1/src/gb/gprs_ns.c File src/gb/gprs_ns.c: PS1, Line 1575: Only the configured remote end may connect "connect" is the system call we issue, and there's no connection on UDP anyway. better: "connect to ensure only we only accept packets from the configured remote end/peer" Line 1579: osmo_sock_init2_ofd(&nsi->nsip.fd, AF_INET, SOCK_DGRAM, we don't really need this indent with our new rule about 120 character long lines. same above for the snprintf line. Please configure your editor in a way to avoid adding line breaks where not needed. PS1, Line 1586: connection packets Line 1590: /* Everyone may connect */ "Accept UDP packets from any source IP/Port" PS1, Line 1597: connection packets -- To view, visit https://gerrit.osmocom.org/4317 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifeb201d9006eec275a46708007ff342cdfc14e45 Gerrit-PatchSet: 1 Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Owner: dexter <[email protected]> Gerrit-Reviewer: Harald Welte <[email protected]> Gerrit-Reviewer: Jenkins Builder Gerrit-HasComments: Yes
