[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH] Replacing (and removing) get_ticks_per_sec() fu

From: Christian Borntraeger
Subject: Re: [Qemu-devel] [PATCH] Replacing (and removing) get_ticks_per_sec() function with NANOSECONDS_PER_SECOND Signed-off-by: Rutuja Shah <address@hidden>
Date: Fri, 11 Mar 2016 12:44:33 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0

On 03/10/2016 08:30 PM, address@hidden wrote:
> From: Rutuja Shah <address@hidden>
> -    s->tick_offset_vmstate = s->tick_offset + delta / get_ticks_per_sec();
> +    s->tick_offset_vmstate = s->tick_offset + delta / NANOSECONDS_PER_SECOND;

While technically correct, I do not like these changes. The interfaces expect 
and the fact that this happens to be a nanosecond does not help regarding 

Lets look at the snippet from above. The variable is called TICK_offset_vmstate.
so, the first line (-) makes sense when reading, the  2nd line (+) does not.
A reader that does not know the timer code will ask itself: "Why do we use 
NANOSECONDS_PER_SECOND to calculate ticks?" The reader has to know that a tick
is a nanosecond to understand that line. So the cleanup makes the code harder to
read in my opinion. 

If - for some reason - we want to replace a function with a MACRO, then
please introduce TICKS_PER_SECOND which just feels better when reading the code.
It would also fix the >80 chars per line problem.
On the other hand get_ticks_per_sec is a static inline function:
- it will get inlined, no overhead over a macro
- it will add type safety if we ever change things (e.g. somebody might 
a tick_t)

So I would prefer to not change things.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]