Attention is currently required from: fixeria, osmith. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-sccp/+/34467?usp=email )
Change subject: sccp: Introduce initial support for SCCP LUDT + LUDTS messages ...................................................................... Patch Set 3: (7 comments) File src/sccp2sua.c: https://gerrit.osmocom.org/c/libosmo-sccp/+/34467/comment/15ffccdc_481ce36a PS3, Line 455: one byte > you say "one", but actually allocating two? Ack https://gerrit.osmocom.org/c/libosmo-sccp/+/34467/comment/f0e77ecf_c43e73df PS3, Line 466: LSB first > use `osmo_store16le()` here? Ack https://gerrit.osmocom.org/c/libosmo-sccp/+/34467/comment/175922a6_ba4286cf PS3, Line 554: ptr_addr + 1 + ptr_value + 2 > Seeing more and more pointer arithmetic, I am wondering if we could have a > packed struct with a unio […] There's a lot of pointer arithmetic because well, the protocol structures work with relative pointer, so pointer arithmetic is needed ;) Moreover, the offsets change depending on the message and/or attribute, so yes, it's a mess no matter how you implement it. https://gerrit.osmocom.org/c/libosmo-sccp/+/34467/comment/eafa22bd_f17c7e14 PS3, Line 557: LSB first > use `osmo_load16le()` here? Ack https://gerrit.osmocom.org/c/libosmo-sccp/+/34467/comment/1497defa_390cf366 PS3, Line 809: ptr_opt + (ptr_opt_is_long ? 1 : 0) > msg->tail > btw, what if the value equals `msg->tail`? I am not 100% sure, but it looks > like you need to make s […] Ack https://gerrit.osmocom.org/c/libosmo-sccp/+/34467/comment/1906a664_caabf4c1 PS3, Line 1415: 254 > but you say 255 in the comment above? That was this way from Harald initial WIP commit. I'll increase it to 255, but anyway all this needs to be further improved to be 100% correct, because according to several parts of spec iirc there's a table saying the maximum size for DATA is lower, but it's all bit more complex, so we can work on that as a follow-up. File tests/xua/xua_test.c: https://gerrit.osmocom.org/c/libosmo-sccp/+/34467/comment/1eb5a4eb_1a8ad2f2 PS3, Line 504: { > `>>` Ack -- To view, visit https://gerrit.osmocom.org/c/libosmo-sccp/+/34467?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-sccp Gerrit-Branch: master Gerrit-Change-Id: Ic91abfc921f5e4f36045bfa325333112cddd9fa6 Gerrit-Change-Number: 34467 Gerrit-PatchSet: 3 Gerrit-Owner: pespin <[email protected]> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <[email protected]> Gerrit-CC: laforge <[email protected]> Gerrit-CC: osmith <[email protected]> Gerrit-Attention: osmith <[email protected]> Gerrit-Attention: fixeria <[email protected]> Gerrit-Comment-Date: Thu, 21 Sep 2023 11:26:21 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: fixeria <[email protected]> Gerrit-MessageType: comment
