Attention is currently required from: jolly.

Patch set 3:Code-Review -1

View Change

10 comments:

• File src/host/layer23/src/mobile/gsm48_rr.c:

  • Patch Set #3, Line 1331: uint8_t *gcr

    `const`

  • Patch Set #3, Line 1387: uint8_t *gcr

    `const`

  • Patch Set #3, Line 1399: uint8_t *gcr, uint8_t *ch_desc

    both `const`

  • Patch Set #3, Line 1440: return -ENOMEM;

    You may be leaking the memory allocated by `asci_notif_alloc()` here.
    Maybe just use `OSMO_ASSERT(nmsg != NULL)`? NULL is unlikely anyway...

  • Patch Set #3, Line 1461: 5*8

    The comment states 36 bits, but you allocate (5 * 8) bytes?
    I guess you need 5 bytes here, or just `OSMO_BYTES_FOR_BITS(36)`.

  • Patch Set #3, Line 1467: 3*8

    Same here, `OSMO_BYTES_FOR_BITS(24)`, because:

    ```
    \param[in] size Number of bytes in the vector
    ```

  • Patch Set #3, Line 1473: \n

    This is going to break the logstring formatting, better use just one `\n`.

  • Patch Set #3, Line 1482: if (chd_bv)

    `bitvec_free()` is NULL safe, so these checks are redundant.

  • Patch Set #3, Line 1494: msgb_l3len(msg) - sizeof(*sgh);

    Is it guaranteed by the caller that `msgb_l3len(msg)` is always `>= sizeof(*sgh)`?
    If not, we may end up with a negative value here and thus huge bitvec length.
    AFAICS, it's not (see my comments to 34529).

  • Patch Set #3, Line 1519: msgb_l3len(msg) - sizeof(*nn);

    Same here.

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