[L] Change in libosmo-sccp[master]: sccp: Introduce initial support for SCCP LUDT + LUDTS messages

Attention is currently required from: fixeria, osmith, pespin. fixeria 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: (8 comments) File src/sccp2sua.c: https://gerrit.osmocom.org/c/libosmo-sccp/+/34467/comment/ccc2fcc4_1a5ca97b PS3, Line 444: * \param[in] iei xUA information element identifier of source data */ `\returns`? https://gerrit.osmocom.org/c/libosmo-sccp/+/34467/comment/f5c5a09b_47911e9a PS3, Line 455: one byte you say "one", but actually allocating two? https://gerrit.osmocom.org/c/libosmo-sccp/+/34467/comment/fa9be1f4_7870b615 PS3, Line 466: LSB first use `osmo_store16le()` here? https://gerrit.osmocom.org/c/libosmo-sccp/+/34467/comment/59dab670_8670c57b 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 union inside maybe? I am not familiar with the protocol and not sure if this approach fits here, but I believe it would improve code readability a lot. https://gerrit.osmocom.org/c/libosmo-sccp/+/34467/comment/6da3f399_9ba247f5 PS3, Line 557: LSB first use `osmo_load16le()` here? https://gerrit.osmocom.org/c/libosmo-sccp/+/34467/comment/88bf1b17_7368d2ef 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 sure that it's less than `msg->tail` (not equal) and that it actually fits: `... > msgb->tail - (ptr_opt_is_long ? 2 : 1)`. https://gerrit.osmocom.org/c/libosmo-sccp/+/34467/comment/16485459_6c35511b PS3, Line 1415: 254 but you say 255 in the comment above? File tests/xua/xua_test.c: https://gerrit.osmocom.org/c/libosmo-sccp/+/34467/comment/eb1a105d_bd15cb9b PS3, Line 504: { `>>` -- 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 <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-CC: laforge <laforge(a)osmocom.org> Gerrit-CC: osmith <osmith(a)sysmocom.de> Gerrit-Attention: osmith <osmith(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Thu, 21 Sep 2023 11:09:31 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment