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: { `>>`