On 10.03.2017 09:38, Paolo Bonzini wrote:
>
>
> 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. :)
Sorry, don't want to get annoying: Yeah, right, so I was eliminating
that variable - do you want me to remove the chunk from the patch?
>> 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. :)
Tried the import, pylint still complains :O
So: Just switch to 3.0 here and in 07/17, and repost?
Ciao,
Stefan
