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

["Lukáš Doktor" <ldoktor@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2357 bytes --]

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 <ldoktor@redhat.com>
>>>> ---
>>>>    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)



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 502 bytes --]