Attention is currently required from: arehbein, pespin, daniel.
fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/33083 )
Change subject: gsm/ipa: Add segmentation callback ......................................................................
Patch Set 9:
(3 comments)
File src/gsm/ipa.c:
https://gerrit.osmocom.org/c/libosmocore/+/33083/comment/6e1c17af_700eabb2 PS2, Line 730: msg->data_len < total_len
Seems better to me, as that would prevent more error cases
Ah, I see. I forgot that `msg->data_len` is the maximum length the buffer can hold, while `msg->len` is how much is used out of `msg->data_len`. Fine then.
File src/gsm/ipa.c:
https://gerrit.osmocom.org/c/libosmocore/+/33083/comment/dcc21e55_f1eb6463 PS8, Line 735: const struct ipaccess_head *hh = (const struct ipaccess_head *) msg->data;
You could declare this at the start of the function (we usually declare variables at the start of th […]
Agreeing with Pau here. There is no strict requirement to declare variables at the top of the respective block, but in this specific case we can improve code readability a bit by doing `msgb_length(msg) < sizeof(*hh)` above.
https://gerrit.osmocom.org/c/libosmocore/+/33083/comment/11bc0665_6da6a3c8 PS8, Line 738: if (msgb_tailroom(msg) + msgb_length(msg) < total_len) {
iiuc the problem here is that the allocated msgb space is not going to be enough to fit in what IPA message expects.
ack.
Talking about that in the log message would be a lot more usefu for the user? since that probably means the dev has to increase the msgb size when allocating it?
Indeed, the logging message should be more specific. Something like `msgb (len=%zu) is not large enough to fit received IPA message (len=%zu)` maybe?
Also, this is not expected to happen too often, so `OSMO_UNLIKELY`?
-
fixeria