Attention is currently required from: Hoernchen. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/30633 )
Change subject: logging: add log level cache ...................................................................... Patch Set 5: (4 comments) Patchset: PS5: thanks, it looks reasonably simple in general, which is good. I am not entirely happy about the frequent #if !EMBEDDED sprinkled around, and I think we can do better by, for example #if-ing out the body of the function log_update_cache(). So the caller sites are unaffected, but the function will turn into a nop as it has an empty body. Same could be done to log_cache_update_all(). Also, I think it would be better if the functions and symbols related to the cache could share a common prefix like log_cache_* - where now we have log_update_cache vs. log_cache_update_all. All not super critical, but I'd hope Eric and/or others agree the code would look cleaner that way. File src/logging.c: https://gerrit.osmocom.org/c/libosmocore/+/30633/comment/09cb298b_c956f3c7 PS5, Line 124: / please add doxygen-style comment desribing function and arguments https://gerrit.osmocom.org/c/libosmocore/+/30633/comment/38b5ad17_7c45d4d6 PS5, Line 1037: log_update_cache this is not guarded by !EMBEDDED, unlike other invocations? https://gerrit.osmocom.org/c/libosmocore/+/30633/comment/e03ebd25_54aca467 PS5, Line 1534: log you just checked it is != NULL above, so that should be a NOP? -- 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: 5 Gerrit-Owner: Hoernchen <[email protected]> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: laforge <[email protected]> Gerrit-Attention: Hoernchen <[email protected]> Gerrit-Comment-Date: Fri, 16 Dec 2022 17:01:40 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
