[M] Change in libosmocore[master]: logging: add 'logging timezone (localtime|utc)'

Attention is currently required from: laforge, fixeria, pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/32043 ) Change subject: logging: add 'logging timezone (localtime|utc)' ...................................................................... Patch Set 1: (8 comments) Patchset: PS1: yes but i figured after converting lots of bools to enums, it would be weird to go on to add a new bool again in something that *could* conceptually have more than two preferences... PS1: (new patch set coming up) File include/osmocom/core/logging.h: https://gerrit.osmocom.org/c/libosmocore/+/32043/comment/2d045bdd_9b9fe8af PS1, Line 400: /* Set timezone for logging of timestamps: 0 for local time, 1 for UTC. Other values are reserved for future right, thx File src/core/logging.c: https://gerrit.osmocom.org/c/libosmocore/+/32043/comment/7cf368c8_78c510f5 PS1, Line 474: return 0; the code before this patch didn't care about the return value of localtime_r(), but if you insist... https://gerrit.osmocom.org/c/libosmocore/+/32043/comment/a14a672d_6516ed7b PS1, Line 528: } else if (target->print_timestamp && get_timestamp(&tv, &tm, target) == 0) { There is an unlikely corner case where it gets called twice, i.e. only if calling the first time failed. In all practical cases it is called once. This code changes to a cleaner switch() in upcoming https://gerrit.osmocom.org/c/libosmocore/+/5857/5/src/core/logging.c#520 https://gerrit.osmocom.org/c/libosmocore/+/32043/comment/f17936bf_4162a716 PS1, Line 874: target->timezone = timezone; i don't agree. i would not want to duplicate some enum validation code here. It can be assumed that the caller only passes valid enum values, and if it is an unknown enum value, then the logging.c switch() deals with that. Otherwise we always have to keep this function "artificially" updated with what logging.c supports. File src/vty/logging_vty.c: https://gerrit.osmocom.org/c/libosmocore/+/32043/comment/89946725_1c316042 PS1, Line 203: log_set_timezone(tgt, LOG_TIMEZONE_UTC); if i can defend above argument, then no here as well =) File tests/logging/logging_vty_test.vty: https://gerrit.osmocom.org/c/libosmocore/+/32043/comment/5058ecde_1a21934f PS1, Line 75: logging_vty_test# logging timestamp ? pau has a solid point here, i failed to add usability tests for the 'logging timestamp' command itself, which now unconvered that i so far fail to write back the new vty command. -- 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: 1 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Tue, 28 Mar 2023 03:27:20 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No 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