Re: [libvirt] [PATCH v2 1/2] timer impl

On 12/14/2010 07:34 PM, Wen Congyang wrote: In addition to Hu's comments, and the fact that you are probably going to revise the exposed interface anyways, here's some additional points. > * src/util/timer.c src/util/timer.h src/util/timer_linux.c src/util/timer_win32.c: > timer implementation > * src/Makefile.am: build timer > * src/libvirt_private.syms: Export public functions > * src/libvirt.c: Initialize timer > * configure.ac: check the functions in librt used by timer > > Signed-off-by: Wen Congyang <wency(a)cn.fujitsu.com&gt; > > > -EXTRA_DIST += util/threads-pthread.c util/threads-win32.c > +EXTRA_DIST += util/threads-pthread.c util/threads-win32.c \ > + util/timer_linux.c timer-win32.c? Also, I'd go with timer-linux.c, not timer_linux.c. > +# timer.h > +get_clock; Bad idea to pollute the namespace with get_clock; better would be something like virGetClock. > +virNewTimer; > +virFreeTimer; > +virModTimer; > +virDelTimer; > > # usb.h > + > +static virTimerPtr timer_list = NULL; > +static void realarm_timer(void); > +static void __realarm_timer(uint64_t); It is dangerous to declare functions in the __ namespace, since that is reserved for libc and friends. > +uint64_t get_clock(void) > +{ > + struct timespec ts; > + > + clock_gettime(CLOCK_MONOTONIC, &ts); > + return ts.tv_sec * 1000000000ULL + ts.tv_nsec; You probably ought to check for overflow here. Dealing with raw nanoseconds is rather fine-grained; is it any better to go with micro or even milliseconds, or does libvirt really require something as precise as nanosecond timeouts?