Attention is currently required from: arehbein, pespin, daniel.

View Change

3 comments:

• File src/gsm/ipa.c:

  • Patch Set #2, 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:

  • Patch Set #8, 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.

  • Patch Set #8, 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`?

To view, visit change 33083. To unsubscribe, or for help writing mail filters, visit settings.