Re: [Qemu-devel] [PATCH 06/11] qmp.py: Couple of pylint/style fixes

[Lukáš Doktor]

Dne 25.7.2017 v 08:04 Philippe Mathieu-Daudé napsal(a):
> Hi Lukáš,
> 
> On 07/24/2017 09:36 AM, Lukáš Doktor wrote:
>> Dne 22.7.2017 v 03:30 Philippe Mathieu-Daudé napsal(a):
>>> Hi Lukáš,
>>>
>>> Since comment/indent fixes and code changes are not related I'd rather see 
>>> this split in at least 2 patches.
>>>
>> Hello Philippe, thank you for the review, I'm wondering what code changes 
>> you have in mind? This is commit should not bring any code changes, just 
>> code reorganization (like including the self.* attributes on top of the file)
>>
>>> On 07/20/2017 01:28 PM, Lukáš Doktor wrote:
>>>> No actual code changes, just a few pylint/style fixes and docstring
>>>> clarifications.
>>>>
>>>> Signed-off-by: Lukáš Doktor <address@hidden>
>>>> ---
>>>>    scripts/qmp/qmp.py | 37 ++++++++++++++++++++++++-------------
>>>>    1 file changed, 24 insertions(+), 13 deletions(-)
>>>>
>>>> diff --git a/scripts/qmp/qmp.py b/scripts/qmp/qmp.py
> [...]
>>>>        def __init__(self, address, server=False, debug=False):
>>>>            """
>>>>            Create a QEMUMonitorProtocol class.
>>>> @@ -42,6 +53,7 @@ class QEMUMonitorProtocol:
>>>>            self.__address = address
>>>>            self._debug = debug
>>>>            self.__sock = self.__get_sock()
>>>> +        self.__sockfile = None
> 
> I was thinking about this line which is new. Now the declaration and 
> initialization are done in __init__() while before it was only 
> declared/initialized in connect() or accept().
> 
> I'm not expert of python interpreter/jit internals but expect the generated 
> code to be slightly different, even if achieving the same.
> 
> not a bit deal, just about wording ;)
> 
Well the difference is that before `connect` you get `AttributeError` when 
looking for `self.__sockfile` while with this patch you'll get `None`. In this 
code nobody queries for `self.__sockfile` before the `connect` so it should not 
change the behavior of this code so I consider that as a `style` fix as it's 
ugly to extend attributes later in code (with some exceptions like 
Namespace-objects..). Anyway if you insist I can split those patches.

Lukáš

>>>>            if server:
>>>>                self.__sock.setsockopt(socket.SOL_SOCKET, 
>>>> socket.SO_REUSEADDR, 1)
>>>>                self.__sock.bind(self.__address)

signature.asc
Description: OpenPGP digital signature