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
--
To view, visit
https://gerrit.osmocom.org/c/libosmocore/+/30633
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I35f8dd9127dd6e7feae392094fd6b3ce2d32558d
Gerrit-Change-Number: 30633
Gerrit-PatchSet: 6
Gerrit-Owner: Hoernchen <ewild(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: Hoernchen <ewild(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Comment-Date: Mon, 19 Dec 2022 09:44:07 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: laforge <laforge(a)osmocom.org>
Gerrit-MessageType: comment