Attention is currently required from: laforge, pespin, fixeria. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/32043 ) Change subject: logging: add 'logging timezone (localtime|utc)' ...................................................................... Patch Set 2: (1 comment) File src/core/logging.c: https://gerrit.osmocom.org/c/libosmocore/+/32043/comment/f2980c3a_c0f6d239 PS1, Line 874: target->timezone = timezone; The bool vs enum aspect doesn't really affect the aspect raised by pespin, does it? Zooming out a bit, the main purpose of this patch is to add a handful of vty script to logging_vty_test.vty. Maybe the complexity is outgrowing the benefit, and we should simply not test the timestamp format in a .vty test? Then we can skip this API entirely. All log_set_..() functions so far just set a config option and return void. It would be a first to add a rc here. Comparing to log_set_category_filter(): if the caller passes an invalid category there, we OSMO_ASSERT() and quit the program. These ideas: If the selected function (gmtime_r() or localtime_r()) was not supported at compile time, then at runtime, one of: - OSMO_ASSERT(false) in logging.c when logging happens - omit timestamps in log output - vty_out("err") and return VTY_CMD_WARNING so starting the application fails. I'd argue for: don't show timestamps. Simple and least adverse effects. Is this important enough for pivoting CN component operation on that? I don't like to add a switch() here that mimicks the switch() in logging.c because it is code dup: switch (target->timezone) { case LOG_TIMEZONE_LOCALTIME: #ifdef HAVE_LOCALTIME_R return 0; #endif case LOG_TIMEZONE_UTC: #ifdef HAVE_GMTIME_R return 0; #endif default: return -1; } -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/32043 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I7f868b47bf8f8dfcf85e735f490ae69b18111af4 Gerrit-Change-Number: 32043 Gerrit-PatchSet: 2 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Wed, 05 Apr 2023 13:42:42 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de> Comment-In-Reply-To: laforge <laforge(a)osmocom.org> Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-MessageType: comment