osmo-bsc[master]: Show OML link uptime in vty

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.

Pau Espin Pedrol gerrit-no-reply at lists.osmocom.org
Mon Sep 25 14:16:46 UTC 2017


Patch Set 3: Code-Review-1

(3 comments)

https://gerrit.osmocom.org/#/c/4009/3/src/libbsc/bsc_vty.c
File src/libbsc/bsc_vty.c:

Line 317: 					sec = (unsigned long long)difftime(tp.tv_sec, bts->uptime);
Make sure bts->uptime is also taken from MONOTONIC clock, otherwise you can get strange values with difftime. You should definetly not mix times using different clock sources. Specially because later on, you use ctime() with bts->uptime. So, as conclusion, one of the two parts of the code is wrong.
I think it makes sense to use a monotonic clock to print the elapsed time, but you should be using a wall clock to print the ctime part, because a monotonic clock doesn't need to contain similar values than a wall clock (which would print really weird dates). This way it can also be seen easily if there has been some type of big clock drift and understand better the real time it has been up.


Strictly speaking I am not sure it's actually good using difftime for this, because according to man page it expects calendar clocks and afaik MONOTONIC clock doesn't follow that rule. Anyway, implementation wise it's only substracting one with another which seems like the expected behaviour.


Line 318: 					vty_out(vty, " %llu days %llu hours %llu min. %llu sec. since %s",
All this code in here looks like a good candidate to be moved to its own function. libosmocore ./include/osmocom/core/timer.h (or a new time_util.h) may be a good idea.

function timespec_to_elapsed(char *buf, timespec *tp) or similar?


Line 319: 						(sec % (60 * 60 * 24 * 365)) / (60 * 60 * 24),
may be nice to also move all this operations to different macros in the same header file? may come handy later in other parts.


-- 
To view, visit https://gerrit.osmocom.org/4009
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I9e4e8504afe8ca467b68d41826f61654e24d9600
Gerrit-PatchSet: 3
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Owner: Max <msuraev at sysmocom.de>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Pau Espin Pedrol <pespin at sysmocom.de>
Gerrit-HasComments: Yes



More information about the gerrit-log mailing list