Attention is currently required from: laforge, msuraev.
fixeria has posted comments on this change. (
https://gerrit.osmocom.org/c/libosmo-sccp/+/29170 )
Change subject: SIGTRAN: add osmo_sccp_tx_disconn_data() helper
......................................................................
Patch Set 2: Code-Review-1
(4 comments)
File examples/sccp_test_vty.c:
https://gerrit.osmocom.org/c/libosmo-sccp/+/29170/comment/a18d7184_e027049c
PS1, Line 97:
unrelated cosmetic change to the indent level. […]
I would expect it to be aligned to the opening brace, and it looks like you intended
to do so. It might be that you're using non-standard tab-size=4, while in C projects
it's 8. Please either undo this change, or properly align to the opening brace.
File examples/sccp_test_vty.c:
https://gerrit.osmocom.org/c/libosmo-sccp/+/29170/comment/7777bfe9_52fc8336
PS2, Line 97: [DATA]
You forgot to add documentation an optional parameter:
# disconnect-req?
disconnect-req N-DISCONNT.req
# disconnect-req ?
<0-16777216> Connection ID
# disconnect-req 0 ?
[DATA] NULL <!-----------------------
https://gerrit.osmocom.org/c/libosmo-sccp/+/29170/comment/8b278161_3277aba5
PS2, Line 103: argv[1]
Is argv[1] guaranteed to be NULL if argc < 2? There might be garbage left in the argv[]
buffer, so I would rather rely on argc:
const uint8_t *data = NULL;
if (argc > 1)
data = (const uint8_t *)argv[1];
Also, wouldn't it be more useful to accept a hex-string as the input?
File include/osmocom/sigtran/sccp_helpers.h:
https://gerrit.osmocom.org/c/libosmo-sccp/+/29170/comment/a07ead1e_14649eeb
PS2, Line 48:
Really weird alignment making it rather harder to read. Tab-size should be 8.
--
To view, visit
https://gerrit.osmocom.org/c/libosmo-sccp/+/29170
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: libosmo-sccp
Gerrit-Branch: master
Gerrit-Change-Id: I92ae22d2cab5863245fba3d904a300055fda34fe
Gerrit-Change-Number: 29170
Gerrit-PatchSet: 2
Gerrit-Owner: msuraev <msuraev(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: msuraev <msuraev(a)sysmocom.de>
Gerrit-Comment-Date: Sun, 21 Aug 2022 17:01:20 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: laforge <laforge(a)osmocom.org>
Gerrit-MessageType: comment