Attention is currently required from: Hoernchen.
neels has posted comments on this change. (
https://gerrit.osmocom.org/c/libosmocore/+/30610 )
Change subject: logging: rework the macros
......................................................................
Patch Set 1: Code-Review-2
(2 comments)
File include/osmocom/core/logging.h:
https://gerrit.osmocom.org/c/libosmocore/+/30610/comment/3b84c434_dcbbf700
PS1, Line 64: if (log_check_level(ss, level)) \
it is a hugely important performance decision to do log_check_level() in the macro,
*before* passing the varargs to logp2(). That is the entire point why log_check_level()
was introduced in this way: if no logging target will print this log message, we can skip
all of the string composition for the log message entirely.
For example, in this:
LOGP(DMAIN, LOGL_DEBUG, "data: %s\n", osmo_hexdump(data, len));
we skip calling osmo_hexdump() when DMAIN's DEBUG is not enabled anywhere.
With your code changes, we would first call osmo_hexdump(), pass the result to
logp2_with_check(), and only then discard the log message. That would be a massive
performance dip for loaded sites where DEBUG logging is switched off.
Now, because log_check_level() is called first, we need to check whether logging was
initialized and redirect to logp_stub() even before that -- log_check_level() just aborts
the program, with assert(osmo_log_info)
File src/logging.c:
https://gerrit.osmocom.org/c/libosmocore/+/30610/comment/afc179d9_a081d87a
PS1, Line 87: struct log_info *osmo_log_info;
(undoing part of earlier patch)
--
To view, visit
https://gerrit.osmocom.org/c/libosmocore/+/30610
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I03efa954cb9e991d2c3da4b61b12aac651e0efa2
Gerrit-Change-Number: 30610
Gerrit-PatchSet: 1
Gerrit-Owner: Hoernchen <ewild(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: Hoernchen <ewild(a)sysmocom.de>
Gerrit-Comment-Date: Thu, 15 Dec 2022 02:32:45 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment