Attention is currently required from: Hoernchen, laforge. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/30633 )
Change subject: logging: add log level cache ......................................................................
Patch Set 6:
(6 comments)
Patchset:
PS6: Agree with Harald regarding common prefix. Also a bit more code documentaton (add more comments) would really help understanding the different pieces for people looking at the logging code.
File src/logging.c:
https://gerrit.osmocom.org/c/libosmocore/+/30633/comment/b86d2f8d_c5c41040 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 descriptive name of what it is.
https://gerrit.osmocom.org/c/libosmocore/+/30633/comment/08d8d16f_8439664c PS6, Line 134: struct log_category tmp = { 99, 0 }; using { .field_a = 99, .field_b = 0} would certainly help here udnerstanding how are you filling this.
https://gerrit.osmocom.org/c/libosmocore/+/30633/comment/ea2c088e_7c399bea PS6, Line 145: log_level_lookup_cache[mapped_subsys] = tmp.enabled ? tmp.loglevel : 99; Please put this 99 in a define. Also some comment explaining what this function implementation does would be nice.
https://gerrit.osmocom.org/c/libosmocore/+/30633/comment/88957624_b0549dbc 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 :)
File tests/bitgen/bitgen_test.c:
https://gerrit.osmocom.org/c/libosmocore/+/30633/comment/f41271aa_6564ed33 PS6, Line 17: SIZE
unrelated changes?
Ack