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

Reply via email to