Attention is currently required from: fixeria.

View Change

2 comments:

• File src/core/logging.c:

  • Patch Set #1, Line 432: int cat_idx

    probably a good idea to make it `unsigned`

    TL;DR: I'll move it to a private header.

    rambling on in case you're interested:

    it ended up this way because

    • intended for private API use only, i'd have liked it to be static, as the commit log explains.
    • at the very heart of the logging subsystem and should rather be lean.
    • When looking at the callers, it is clear that it's correct.

    Yet I did think about it quite a bit:

    we actually do have negative categories, the DL* ones.
    so i did ask myself, should it convert from one to the other?

    But:
    It is called only in places where the idx is already checked,
    and the local variable at all the callers is an int...

    So maybe in future it should be able to convert a negative category?
    Considering these back and forth I ended up at "good enough compromise, not important enough to bother"...

    But your remark makes me reconsider, I guess it should be less public?

  • Patch Set #1, Line 434: const char *name = log_info->cat[cat_idx].name;

    add `OSMO_ASSERT(cat_idx < log_info->num_cat)`?

    (as above)

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