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?