On 10.03.2017 09:14, Paolo Bonzini wrote:
>
>
> ----- Original Message -----
>> From: "Stefan Raspl" <raspl@xxxxxxxxxxxxxxxxxx>
>> To: "Paolo Bonzini" <pbonzini@xxxxxxxxxx>, kvm@xxxxxxxxxxxxxxx
>> Cc: rkrcmar@xxxxxxxxxx, frankja@xxxxxxxxxxxxxxxxxx
>> Sent: Friday, March 10, 2017 7:04:46 AM
>> Subject: Re: [PATCH 04/17] tools/kvm_stat: fix misc glitches
>>
>> On 09.03.2017 17:51, Paolo Bonzini wrote:
>>>
>>>
>>> On 20/02/2017 16:41, Stefan Raspl wrote:
>>>> @@ -339,8 +338,7 @@ def get_filters():
>>>> filters['kvm_exit'] = ('exit_reason', ARCH.exit_reasons)
>>>> return filters
>>>>
>>>> -libc = ctypes.CDLL('libc.so.6', use_errno=True)
>>>> -syscall = libc.syscall
>>>> +syscall = ctypes.CDLL('libc.so.6', use_errno=True).syscall
>>>>
>>>> class perf_event_attr(ctypes.Structure):
>>>> """Struct that holds the necessary data to set up a trace event.
>>>> @@ -950,11 +948,10 @@ class Tui(object):
>>>> while True:
>>>> self.refresh(sleeptime)
>>>> curses.halfdelay(int(sleeptime * 10))
>>>> - sleeptime = 3
>>>> + sleeptime = 3.
>>>> try:
>>>> char = self.screen.getkey()
>>>> if char == 'x':
>>>> - self.drilldown = not self.drilldown
>>>> self.update_drilldown()
>>>> if char == 'q':
>>>> break
>>>
>>> I'm not sure I understand the point of these; the rest is fine.
>>
>> 'sleeptime' starts out as a float (sleeptime = 0.25), but is here
>> re-defined to an int - so we make it float all the way.
>> The variable 'drilldown' is never used, so we remove its
>> initialization in __init__() and the sole place where it is ever
>> used, which is the line above.
>
> Yes, I was referring to libc and sleeptime. I don't like the
> "3.", especially since Python 3 has "3/2" return a float. Is
> this a PEP8 complaint? Does it still complain if you do
> "from __future__ import division"?
Ah, sorry, missed the first chunk: That was to eliminate unused
variable 'libc'.
As for the "3.": I noticed through pylint, PEP8 is fine. Seemed
valid to me, but in the grand scheme of things it certainly doesn't
matter (and wasn't aware it becomes moot in Python 3), so I'd take
it out if you want.
Sidenote: I only ever hear the complaint about the '3.' from kernel
folks, seemed quite common to me elsewhere - maybe 'cause there's
usually no floats in the kernel?! But as I said, I'd rather take it
out altogether in here instead of adding more imports, and fix 07/17
to redefine to 3 (instead of 3.), OK?
Ciao,
Stefan
