Attention is currently required from: neels, laforge, dexter.
jolly has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-msc/+/33511 )
Change subject: ASCI: Add call control for VGCS/VBS ......................................................................
Patch Set 15:
(10 comments)
File src/libmsc/msc_vgcs.c:
https://gerrit.osmocom.org/c/osmo-msc/+/33511/comment/942d0edb_d3f30f83 PS14, Line 179: OSMO_ASSERT(l3_msg);
this would abort the program on an encoding error? […]
Done
https://gerrit.osmocom.org/c/osmo-msc/+/33511/comment/81731566_381cbb13 PS14, Line 231: *callref = ntohl(*(uint32_t *)ie) >> 5;
maybe better use osmo_load32be() because it also works reliably when the pointer is not on a word bo […]
Done
https://gerrit.osmocom.org/c/osmo-msc/+/33511/comment/a0d9d210_a6fd0d95 PS14, Line 801: if (msc_a) {
(early exit pattern: 'if (!msc_a) return;' means less indent. […]
Done
https://gerrit.osmocom.org/c/osmo-msc/+/33511/comment/5f927165_643c96c3 PS14, Line 1628: OSMO_ASSERT(l3_msg);
encoding error: do not abort the program, log and return error
Done
https://gerrit.osmocom.org/c/osmo-msc/+/33511/comment/1f15e3fe_4a57750c PS14, Line 1666: OSMO_ASSERT
.
Done
https://gerrit.osmocom.org/c/osmo-msc/+/33511/comment/556aa72f_b079cfe5 PS14, Line 1727: osmo_fsm_inst_free(bss->fi);
rather use osmo_fsm_inst_term(). It terminates the instance gracefully and then frees it. […]
Done
https://gerrit.osmocom.org/c/osmo-msc/+/33511/comment/a2e129d8_36b77257 PS14, Line 1730: talloc_free(bss);
(hint, in case you like it too, i like to allocate the struct as a talloc child of the FSM instance, […]
Ok, makes sense. (Alternatively one could do the talloc_free inside the destructor of the the FSM, if the FSM is a child of the allocated structure.)
https://gerrit.osmocom.org/c/osmo-msc/+/33511/comment/379da719_327d5bc2 PS14, Line 1796: OSMO_ASSERT
.
Done
https://gerrit.osmocom.org/c/osmo-msc/+/33511/comment/ceaf9210_79a37efc PS14, Line 1938: OSMO_ASSERT
.
Done
https://gerrit.osmocom.org/c/osmo-msc/+/33511/comment/9a2c5337_477a8306 PS14, Line 1949: OSMO_ASSERT
.
Done