Attention is currently required from: iedemam.

View Change

5 comments:

• File include/osmocom/bsc/gsm_data.h:

  • Patch Set #13, Line 771: struct osmo_time_cc timekeeper;

    here is your problem: the lchan is completely cleared out every time an lchan is activated / unused. See lchan_reset(), there it loses its pointer to the ctr. In lchan_reset(), you need to make sure the timekeeper stays unchanged by adding

      .timekeeper = lchan->timekeeper,

    lchan->activate is intended as a volatile place describing the state of a single activation. Rather put your timekeeper in the "lchan root" please.

    Nitpick on the name: let's call it 'active_ms' or 'active_time' or something more concise.

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

  • Patch Set #13, Line 354: !&lchan->activate.timekeeper

    a) the address of a struct member is *always* non-NULL. (ok if a struct pointer is NULL and the member is the first in the struct, it could be NULL, but that's beside the point)

    b) even if you drop the '&', timekeeper is not a pointer.

    This condition will always evaluate false. the compiler should have told you that, too, wondering why jenkins passes the build.
    Completely drop this 'if'

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

  • Patch Set #13, Line 228: .gran_usec = 1*1000000,

    if you want milliseconds, 1000 is your factor. This says after how much active time the rate counter increments; using one second for that does make sense, just saying. The total_sum is always in microseconds regardless, so probably this is correct after all.

  • Patch Set #13, Line 237: /* TMP HACK: this fixes "make check" handover tests... */

    yeah because you re-init the lost state after lchan_reset().

  • Patch Set #13, Line 558: if (&lchan->activate.timekeeper) {

    this condition is never false, as above. simply set the the flag without condition.

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