Attention is currently required from: falconia.

Patch set 3:Code-Review +1

View Change

1 comment:

• File src/common/l1sap.c:

  • Patch Set #2, Line 2098: switch (lchan->tch_mode) {

    the only valid modes are 6.0k and 3.6k raw rates, and my code does OSMO_ASSERT on all others. Changing the code to use csd_v110_lchan_desc[] would entail allowing invalid modes [...]

    The point of using `csd_v110_lchan_desc[]` is to avoid using magic numbers, which as we have already seen [in the case of 26] raising questions like "where it's coming from?". You can keep the switch statement if really want to be 100% sure that we were given the right lchan kind:

    ```
    const struct csd_v110_lchan_desc *desc;

    switch (lchan->tch_mode) {
    case GSM48_CMODE_DATA_6k0:
    case GSM48_CMODE_DATA_3k6:
        desc = csd_v110_lchan_desc[lchan->tch_mode];
        bits_per_20ms = desc->num_blocks * desc->num_bits;
        break;
    default:
        OSMO_ASSERT(0);
    }
    ```

    The reason why I am suggesting to move `csd_v110_lchan_desc[]` to libosmocore.git is that we also want to use those definitions in osmocom-bb.git/trxcon (we do have a similar lookup table in there). This is of course not a blocker and can be done later separately from this patch.

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