Attention is currently required from: neels, laforge, pespin, fixeria.
Max has posted comments on this change. (
https://gerrit.osmocom.org/c/libosmo-sccp/+/29084 )
Change subject: SIGTRAN: cache Optional Data for SCCP CR/CC/RLSD
......................................................................
Patch Set 15:
(8 comments)
File src/sccp_scoc.c:
https://gerrit.osmocom.org/c/libosmo-sccp/+/29084/comment/4363d4d6_ffda9ece
PS15, Line 642: /* optional: importance */
why is this line removed here? this is non related.
The RLC does not have optional importance field.
https://gerrit.osmocom.org/c/libosmo-sccp/+/29084/comment/b870683f_95186f62
PS15, Line 600: if (
Ack
We shouldn't. Shall I replace it with
assert or just drop?
https://gerrit.osmocom.org/c/libosmo-sccp/+/29084/comment/6fa8a826_fd35be4a
PS15, Line 606: } else
remove curly braces in the if.
Actually it's
better to add them to else according to
https://www.kernel.org/doc/html/latest/process/coding-style.html#placing-br…
https://gerrit.osmocom.org/c/libosmo-sccp/+/29084/comment/51617eeb_e7923d15
PS15, Line 630: if (xua_drop_data_check(prim, SCCP_MAX_DATA, "cache overrun"))
... […]
Done
https://gerrit.osmocom.org/c/libosmo-sccp/+/29084/comment/f4acfafc_5ad77dad
PS15, Line 635: LOGP(DLSCCP, LOGL_ERROR, "replacing unsent %u bytes of optional
data cache with %s optional data\n",
NOTICE
I think it's error - this situation
should not arise normally.
https://gerrit.osmocom.org/c/libosmo-sccp/+/29084/comment/da5d2afe_a69997e6
PS15, Line 639: msgb_alloc(SCCP_MAX_DATA, "SCCP optional data cache for
CR/CC/RLSD");
Ack
Done
https://gerrit.osmocom.org/c/libosmo-sccp/+/29084/comment/5d2bd69d_a2630137
PS15, Line 639: msgb_alloc(SCCP_MAX_DATA, "SCCP optional data cache for
CR/CC/RLSD");
Ack
Done
https://gerrit.osmocom.org/c/libosmo-sccp/+/29084/comment/699f9d67_3b1a53c1
PS15, Line 784: xua_msg_add_sccp_addr(xua, SUA_IEI_DEST_ADDR,
&conn->calling_addr);
why is this line removed here? this is non related.
Similar to the above: the comments are placed to match the fields order in the
spec.
--
To view, visit
https://gerrit.osmocom.org/c/libosmo-sccp/+/29084
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: libosmo-sccp
Gerrit-Branch: master
Gerrit-Change-Id: I0033faf9da393418930252233ce74d62cd1cef8a
Gerrit-Change-Number: 29084
Gerrit-PatchSet: 15
Gerrit-Owner: Max <msuraev(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Comment-Date: Tue, 30 Aug 2022 16:07:09 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: laforge <laforge(a)osmocom.org>
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: comment