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

View Change

2 comments:

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

  • Patch Set #9, Line 1744: unsigned long ny1_recommended = (T3124 + GSM_NY1_REQ_DELTA)/T3105 + 1;

    I'm not sure I understand... […]

    I'm only saying this as a suggestion, in case you are aiming for a clean ceil() instead of a "custom" calculation like floor() + 1.

    I'm not sure what's the correct formula, but the comment sounds like you wanted a rounded up integer division, as in

      ceil((float)(T3124 + GSM_NY1_REQ_DELTA) / T3105)

    if that is true then the correct integer division here is

      (T3124 + GSM_NY1_REQ_DELTA + (T3105-1)) / T3105

    (like in OSMO_BYTES_FOR_BITS() for example)

    The current patch results in

       floor((float)(T3124 + GSM_NY1_REQ_DELTA) / T3105) + 1

    which is one too many if the numbers work out as a clean division (no fractional part).

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

  • Patch Set #14, Line 1738: unsigned long T3105 = osmo_tdef_get(bts->network->T_defs, 3105, OSMO_TDEF_MS, -1);

    that was my 'reasoning' as well, didn't see it in our guidelines. […]

    Maybe this rule is implied by "linux kernel style".

    All i know is that code review from day one of submitting osmocom patches asked me to put vars at the top, the reasoning being supposedly compatibility with compilers, and i adopted that (though i personally very much prefer declaring vars at their actual first use)

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