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-bra...
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.