Patch Set 4: (2 comments)
https://gerrit.osmocom.org/#/c/5103/4/src/timer.c File src/timer.c: Line 202: if (clock_gettime(CLOCK_MONOTONIC, &t_now) != 0) { some while ago, we added a clock wrapper to be able to set artificial times for unit tests. Have you made sure that applying this to gprs_ns in the next patch isn't breaking any tests that rely on artificial time? I guess it would be nice to provide a fake-time wrapper around monotonic clocks as well for unit tests. (which would also serve to test this function itself, see below) Line 214: return OSMO_SEC2MS(sec) + OSMO_NSEC2MS(ns); are you sure this is actually correct? from = 1s 500000ns t_now = 2s 490000ns OSMO_SEC2MS(2-1) = 1 * 1000 OSMO_NSEC2MS(490k - 500k) = -10k / 1000 return 1000 - 10 ok seems legit ... still would be nice to have it unit testable and go through various cases there. -- To view, visit https://gerrit.osmocom.org/5103 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I83d865ff633a7ebda2c943477205fd31aceda277 Gerrit-PatchSet: 4 Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Owner: Max <msur...@sysmocom.de> Gerrit-Reviewer: Harald Welte <lafo...@gnumonks.org> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Neels Hofmeyr <nhofm...@sysmocom.de> Gerrit-Reviewer: Vadim Yanitskiy <axilira...@gmail.com> Gerrit-HasComments: Yes