On 10/03/2017 09:33, Stefan Raspl wrote:
> 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'.
It's not unused, it's used on the following line. :)
> 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.
I think it's a valid complaint with Python 2 division. If pylint
complaints even with "from __future__ import division", it would be a
pylint bug in my opinion.
Maybe it's just me... I wouldn't have complained about "3.0" for example. :)
Paolo
> 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?
