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;

Going for the bool value local vs. UTC might avoid the entire isue altogether. […]

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; }