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