Attention is currently required from: fixeria.

View Change

3 comments:

• File src/gprs_ms.c:

  • Patch Set #2, Line 355: OSMO_ASSERT(ms);

    Somehow it feels wrong when I am seeing a function assert()ing all its pointer arguments. […]

    it's not really the same. These are internal pointers, not stuff received from the peers over the wire. They are checking internal state, and help understand readers (and remind myself) of the expected assumptions.

  • Patch Set #2, Line 409: OSMO_ASSERT(!ms_tbf_is_attached(ms, tbf));

    ... especially when some calling functions also do assert the arguments too.

    Again, this is internal state. If it breaks it means something is fundamentally wrong in the model, or somebody made a change in the wrong direction. It also helps understand the current model.

  • Patch Set #2, Line 419: OSMO_ASSERT(tbf_ms(tbf) == ms);

    ... and here you're dereferencing tbf before assert()ing it.

    I can add an OSMO_ASSERT(tbf) there if you want, but it's clear here that it is expected to be not NULL.

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