Attention is currently required from: neels, pespin, fixeria.

View Change

7 comments:

• File src/osmo-bsc/bts.c:

  • Patch Set #9, Line 1729: struct gsm_lchan *gl = gsm_bts_get_cbch(bts);

    According to the specs, T3124 is set depending on the channel type (I assume 'lchan' stands for 'lin […]

    Done

• File src/osmo-bsc/bts.c:

  • Patch Set #18, Line 535: if (!gsm_bts_check_ny1(bts, LOGL_ERROR)) return -EINVAL;

    put return always in a separate line.

    Done

  • Patch Set #18, Line 1726: const struct gsm_lchan *gl = gsm_bts_get_cbch(bts);

    I'm still not understanding why are you taking a CBCH channel here, seems totally not related. […]

    I removed the check.

  • Patch Set #18, Line 1729: if (gl) {

    no need for {} here. Linter should already have warned about it.

    I used 'lint_diff.sh' from our Docker tests locally (I think that's where I got it from) and didn't get anything. Looking at it again, I guess I have to run it after creating the commit (seems like it runs on the latest commit, not on local changes, which I was assuming).

    Apart from that, does Jenkins fail if the linter warns?

  • Patch Set #18, Line 1730: T3124 = (gl->type == GSM_LCHAN_SDCCH) ? GSM_T3124_SDCCH : GSM_T3124_OTHER_CH;

    this is always going to be false since gl is CBCH.

    Done

  • Patch Set #18, Line 1732: /* Take the higher lower bound to be safe */

    you can then move the comment to the "else" line (1731)

    Done

• File src/osmo-bsc/bts_ipaccess_nanobts_omlattr.c:

  • Patch Set #18, Line 95: gsm_bts_check_ny1(bts, LOGL_NOTICE);

    I really see no reason for having different log levels.

    This is the additional check during runtime (vs. startup), so should this function also exit with an error if the check isn't successful? My thinking was if we don't exit, it shouldn't count as an error message.

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