Attention is currently required from: fixeria, lynxis lazus, pespin.
dexter has posted comments on this change by dexter. ( https://gerrit.osmocom.org/c/osmo-sgsn/+/43958?usp=email )
Change subject: gprs_sndcp_xid: replace OSMO_ASSERT with if and return ......................................................................
Patch Set 3:
(4 comments)
File src/sgsn/gprs_sndcp_xid.c:
https://gerrit.osmocom.org/c/osmo-sgsn/+/43958/comment/4f557e59_cab00ecc?usp... : PS2, Line 293: /* Encode P0 (see also: 3GPP TS 44.065, 6.6.2.1, Table 7a) */
For another commit: […]
Its covered by the check above, but I agree, its a bit difficult to see. I will add another check in a follow up patch, so that we are still safe in case encode_pcomp_applicable_sapis writes more than 2 bytes - maybe when the spec gets upgraded and we do not notice, its a real risk...
https://gerrit.osmocom.org/c/osmo-sgsn/+/43958/comment/d928c681_b10a4054?usp... : PS2, Line 379: if (params->p3t < 0 || params->p3t > 65535 || params->p3t < 2 * params->p1t)
better enclose (2*params->p1t) into parenthesis? also below in line 388.
Done
https://gerrit.osmocom.org/c/osmo-sgsn/+/43958/comment/ed9dceee_e3ff9449?usp... : PS2, Line 623: } else if (rc > 0) {
no need for "} else" here (early return), remove it.
But we still need the If then, because we do not want to do the tlv_put when the data length (rc) is 0.
https://gerrit.osmocom.org/c/osmo-sgsn/+/43958/comment/44aab306_22493fdd?usp... : PS2, Line 636: } else if (rc > 0) {
no need for "} else" here (early return), remove it.
Done
-
dexter