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

[Jan Kiszka <jan.kiszka@siemens.com>]

Juan Quintela wrote:
> Jan Kiszka <jan.kiszka@siemens.com> wrote:
>> Juan Quintela wrote:
>>> Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>> Juan Quintela wrote:
>>>>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>>>> [...]
>>>>
>>>>> 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.

Ah, ok, misunderstanding on my side (I'm a bit slow today). Will file a
patch on top of yours.

Jan

-- 
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux