[Qemu-devel] Re: [PATCH 05/26] Unexport ticks_per_sec variable. Create get_ticks_per_sec() function

[Juan Quintela]

Jan Kiszka <address@hidden> wrote:
> Juan Quintela wrote:
>> Jan Kiszka <address@hidden> wrote:
>>> Juan Quintela wrote:
>>>> Signed-off-by: Juan Quintela <address@hidden>
>>> [...]
>>>
>>>> diff --git a/vl.c b/vl.c
>>>> index 448ec6c..33abef2 100644
>>>> --- a/vl.c
>>>> +++ b/vl.c
>>>> @@ -186,7 +186,7 @@ enum vga_retrace_method vga_retrace_method = 
>>>> VGA_RETRACE_DUMB;
>>>>  static DisplayState *display_state;
>>>>  DisplayType display_type = DT_DEFAULT;
>>>>  const char* keyboard_layout = NULL;
>>>> -int64_t ticks_per_sec;
>>>> +static int64_t ticks_per_sec;
>>>>  ram_addr_t ram_size;
>>>>  int nb_nics;
>>>>  NICInfo nd_table[MAX_NICS];
>>>> @@ -1032,6 +1032,11 @@ int64_t qemu_get_clock(QEMUClock *clock)
>>>>      }
>>>>  }
>>>>
>>>> +int64_t get_ticks_per_sec(void)
>>>> +{
>>>> +    return ticks_per_sec;
>>>> +}
>>>> +
>>> This refactoring would be even more useful if that one became
>>>
>>> static inline int64_t get_ticks_per_sec(void)
>>> {
>>>     return QEMU_CLOCK_BASE;
>>> }
>>>
>>> Right now we don't have a use for the ticks_per_sec /variable/, it's
>>> always constant. Your interface does not prevent changing this in the
>>> future, though, which is just like it should be.
>> 
>> I liked this one on top.
>
> Why?

Because that is how we use it.  Mine is refactoring, yours is changing
behavior.  With my patch, it is still possible to change ticks_per_sec
(not that this is a good idea, but that is how the code is today).  With
yours, you remove that feature.  I fully agree with your change.
In my book that are _two_ patches.

Later, Juan.