Provide a build time switch to disable logging in optimized builds

[kaie]

During execution with real world keys, RNP dumps lots of warnings on the console, apparently from RNP_LOG

Could you please provide a mechanism to disable those in an optimized build?

[signature_parse_subpacket() /home/user/moz/commcent/mozilla/comm/third_party/rnp/src/librepgp/stream-packet.cpp:1420] unknown subpacket : 101
[signature_check_certification() /home/user/moz/commcent/mozilla/comm/third_party/rnp/src/librepgp/stream-sig.cpp:1203] key expired 3065799 seconds ago
[signature_check_binding() /home/user/moz/commcent/mozilla/comm/third_party/rnp/src/librepgp/stream-sig.cpp:1239] subkey expired 71184103 seconds ago
[signature_check() /home/user/moz/commcent/mozilla/comm/third_party/rnp/src/librepgp/stream-sig.cpp:1155] signature made after key expiration
[signed_src_finish() /home/user/moz/commcent/mozilla/comm/third_party/rnp/src/librepgp/stream-parse.cpp:936] signer's key not found

[ni4]

Sure, while it is easy to completely disable this (or just remove some noisy lines), shouldn't we have some sort of fallback to be able to debug later on? I.e. to have some data for the users to send it back.

[kaie]

Maybe check for an environment variable at startup, and remember the flag during the runtime? If set, then log.

[ni4]

Yeah, this sounds as a solution. What do you think, @dewyatt? cc @ronaldtse, @rrrooommmaaa

[kaie]

#ifdef DEBUG then log
#else
if getenv("RNP_LOG_CONSOLE") then log
else silence
#endif

[ni4]

My only concern here is performance in case of large amount of log messages. Probably better to keep some global inner state variable.

[ronaldtse]

Probably better to keep some global inner state variable.

Agree with @ni4 .

[dewyatt]

Right we may want to avoid excessive getenv invocations. So I guess some lazily-initialized global is the quick & easy solution. (And at some point we ought to avoid globals and encapsulate things like this in rnp_ffi_t, etc) ‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐

…

On Monday, April 20, 2020 6:01 AM, Nickolay Olshevsky ***@***.***> wrote: My only concern here is performance in case of large amount of log messages. Probably better to keep some global inner state variable. — You are receiving this because you were mentioned. Reply to this email directly, [view it on GitHub](#1103 (comment)), or [unsubscribe](https://github.com/notifications/unsubscribe-auth/AAFNXYKPNXE7YUNXNWNY6BLRNQMPFANCNFSM4MLY3PNQ).

[ni4]

Okay, so let's stick to this strategy:

• for Debug builds logging is always on
• for Release builds logging is enabled (storing status in some global variable) only if RNP_LOG_CONSOLE environment variable is set.
• we'll need to have some small test for this as well.

@rrrooommmaaa Would you like to take this issue?

[ronaldtse]

Anyone interested in this? We need to get this going. Thanks!

Ping @rrrooommmaaa @joke325 .