Attention is currently required from: wbokslag.

View Change

6 comments:

• File src/lower_mac/tetra_lower_mac.c:

  • Patch Set #2, Line 347: msg->len = msg

    in general we don't ever manually manipulate msgb's this way in other osmocom software. We use the various msgb.[ch] functions as high-level operations on the buffer (like msgb_put/pull/push/get/trim/reserve/...), but don't consider manipulation of the individual fields as good code.

    I don't know the specific use case here (and have too many other tasks to review in detail) so I don't have a concrete suggestion, just sharing general thoughts.

• File src/tetra_llc.c:

  • Patch Set #2, Line 124:

    	msg->tail = msg->l3h + lpp.tl_sdu_len; // Strips off FCS (if present)
    	msg->len = msg->tail - msg->head;
    

    this looks a bit like openn-coding a msgb_get(), or probably even more a msgb_l3trim() ?

• File src/tetra_upper_mac.c:

  • Patch Set #2, Line 178: msg->len = m

    same here. msgb_trim?

  • Patch Set #2, Line 185: msg->len = m

    same here. msgb_trim?

  • Patch Set #2, Line 306: msg->len = m

    yet another msgb_trim? Also, the entire get_num_fill_bits + following operations happens several times in the code, so it might make sense to create a function to cover the sequence of getting the number of fill-bits and then trimming the message to exclude those?

  • Patch Set #2, Line 359: msg->len = m

    again another fill-bits-stripping situation.

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