Attention is currently required from: laforge, pespin. Hoernchen has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/30633 )
Change subject: logging: add log level cache ......................................................................
Patch Set 7:
(5 comments)
Patchset:
PS5:
thanks for trying and explaining. […]
I would rather keep all those functions within the current !embedded define area instead of deliberately moving those function outside just to get the optimizer to remove those.. They should not be used or exist for embedded targets anyway, so the current way is safer, because embedded builds would error due to dangling symbols.
File src/logging.c:
https://gerrit.osmocom.org/c/libosmocore/+/30633/comment/15b803f1_c5f5eebe PS6, Line 107: memset(tmp_level, 100, osmo_log_info->num_cat);
It would be great to have this 100 and 99 which I see used below defined with a macro, holding a des […]
Done
https://gerrit.osmocom.org/c/libosmocore/+/30633/comment/1a1e2053_55b31dd9 PS6, Line 134: struct log_category tmp = { 99, 0 };
using { .field_a = 99, . […]
Done
https://gerrit.osmocom.org/c/libosmocore/+/30633/comment/8cb45442_fc7ceee4 PS6, Line 145: log_level_lookup_cache[mapped_subsys] = tmp.enabled ? tmp.loglevel : 99;
Please put this 99 in a define. […]
Done
https://gerrit.osmocom.org/c/libosmocore/+/30633/comment/32e1dd7b_4e75c1dc PS6, Line 156: return (level < log_level_lookup_cache[mapped_subsys]) ? false : true;
"return (log_level_lookup_cache[mapped_subsys] >= level)" seems a lot shorter :)
This is precisely the inverted "when not to log" approach used by the existing check and I am reusing the style that is already present in the code because I hoped that by doing that I would not get into these kind ofs discussions.