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 <[email protected]> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <[email protected]> Gerrit-CC: pespin <[email protected]> Gerrit-Attention: Hoernchen <[email protected]> Gerrit-Attention: laforge <[email protected]> Gerrit-Comment-Date: Mon, 19 Dec 2022 09:44:07 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: laforge <[email protected]> Gerrit-MessageType: comment
