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