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 <[email protected]> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <[email protected]> Gerrit-Reviewer: laforge <[email protected]> Gerrit-Attention: laforge <[email protected]> Gerrit-Attention: msuraev <[email protected]> Gerrit-Comment-Date: Sun, 21 Aug 2022 17:01:20 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: laforge <[email protected]> Gerrit-MessageType: comment
