2020-04-12 14:39:21

by Jarkko Sakkinen

[permalink] [raw]
Subject: [PATCH] selftests/tpm2: Change exception handling to be Python 3 compatible

I need more time to fix all the byte array / string related stuff but
it makes sense to fix the exceptions as it is fairly mechanical
procedure:

1,$s/except \(.*\), \(.*\):/except \1(\2):/g

I.e. fix the low hanging fruit first and the rest later.

Signed-off-by: Jarkko Sakkinen <[email protected]>
---
tools/testing/selftests/tpm2/tpm2_tests.py | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/tools/testing/selftests/tpm2/tpm2_tests.py b/tools/testing/selftests/tpm2/tpm2_tests.py
index 728be7c69b76..c3c06899e042 100644
--- a/tools/testing/selftests/tpm2/tpm2_tests.py
+++ b/tools/testing/selftests/tpm2/tpm2_tests.py
@@ -65,7 +65,7 @@ class SmokeTest(unittest.TestCase):
blob = self.client.seal(self.root_key, data, auth, None)
try:
result = self.client.unseal(self.root_key, blob, auth[:-1] + 'B', None)
- except ProtocolError, e:
+ except ProtocolError(e):
rc = e.rc

self.assertEqual(rc, tpm2.TPM2_RC_AUTH_FAIL)
@@ -119,7 +119,7 @@ class SmokeTest(unittest.TestCase):
self.client.policy_password(handle)

result = self.client.unseal(self.root_key, blob, auth, handle)
- except ProtocolError, e:
+ except ProtocolError(e):
rc = e.rc
self.client.flush_context(handle)
except:
@@ -136,7 +136,7 @@ class SmokeTest(unittest.TestCase):
rc = 0
try:
blob = self.client.seal(self.root_key, data, auth, None)
- except ProtocolError, e:
+ except ProtocolError(e):
rc = e.rc

self.assertEqual(rc, tpm2.TPM2_RC_SIZE)
@@ -152,7 +152,7 @@ class SmokeTest(unittest.TestCase):
0xDEADBEEF)

self.client.send_cmd(cmd)
- except IOError, e:
+ except IOError(e):
rejected = True
except:
pass
@@ -212,7 +212,7 @@ class SmokeTest(unittest.TestCase):
self.client.tpm.write(cmd)
rsp = self.client.tpm.read()

- except IOError, e:
+ except IOError(e):
# read the response
rsp = self.client.tpm.read()
rejected = True
@@ -283,7 +283,7 @@ class SpaceTest(unittest.TestCase):
rc = 0
try:
space1.send_cmd(cmd)
- except ProtocolError, e:
+ except ProtocolError(e):
rc = e.rc

self.assertEqual(rc, tpm2.TPM2_RC_COMMAND_CODE |
--
2.25.1


2020-04-12 15:03:36

by Ezra Buehler

[permalink] [raw]
Subject: Re: [PATCH] selftests/tpm2: Change exception handling to be Python 3 compatible

Hi Jarkkon,

> On 12 Apr 2020, at 16:36, Jarkko Sakkinen <[email protected]> wrote:
> + except ProtocolError(e):

Should this not be

except ProtocolError as e:

?

Cheers,
Ezra.

2020-04-12 17:20:59

by Jarkko Sakkinen

[permalink] [raw]
Subject: Re: [PATCH] selftests/tpm2: Change exception handling to be Python 3 compatible

On Sun, Apr 12, 2020 at 05:02:27PM +0200, Ezra Buehler wrote:
> Hi Jarkkon,
>
> > On 12 Apr 2020, at 16:36, Jarkko Sakkinen <[email protected]> wrote:
> > + except ProtocolError(e):
>
> Should this not be
>
> except ProtocolError as e:

Unless there is a functional difference, does it matter?

/Jarkko

2020-04-13 05:55:38

by Ezra Buehler

[permalink] [raw]
Subject: Re: [PATCH] selftests/tpm2: Change exception handling to be Python 3 compatible

Hi Jarkko,

On 12 Apr 2020, at 19:07, Jarkko Sakkinen <[email protected]> wrote:
>
> On Sun, Apr 12, 2020 at 05:02:27PM +0200, Ezra Buehler wrote:
>> Hi Jarkkon,
>>
>>> On 12 Apr 2020, at 16:36, Jarkko Sakkinen <[email protected]> wrote:
>>> + except ProtocolError(e):
>>
>> Should this not be
>>
>> except ProtocolError as e:
>
> Unless there is a functional difference, does it matter?
>
> /Jarkko

Well, your patch confuses me a lot. It looks to me like you are passing
the undefined `e` variable to the constructor.

When I run flake8 on it I get following error (among others):

F821 undefined name 'e'

What I suggested is the standard syntax:
https://docs.python.org/3/tutorial/errors.html

Did you test this? You should get an error as soon as an exception
occurs.

Cheers,
Ezra.

2020-04-14 14:31:33

by Ezra Buehler

[permalink] [raw]
Subject: Re: [PATCH] selftests/tpm2: Change exception handling to be Python 3 compatible

Hi Jarkko,

On 13 Apr 2020, at 20:04, Jarkko Sakkinen <[email protected]> wrote:
>
> On Mon, Apr 13, 2020 at 07:02:20AM +0200, Ezra Buehler wrote:
>> Hi Jarkko,
>>
>> On 12 Apr 2020, at 19:07, Jarkko Sakkinen <[email protected]> wrote:
>>>
>>> On Sun, Apr 12, 2020 at 05:02:27PM +0200, Ezra Buehler wrote:
>>>> Hi Jarkkon,
>>>>
>>>>> On 12 Apr 2020, at 16:36, Jarkko Sakkinen <[email protected]> wrote:
>>>>> + except ProtocolError(e):
>>>>
>>>> Should this not be
>>>>
>>>> except ProtocolError as e:
>>>
>>> Unless there is a functional difference, does it matter?
>>>
>>> /Jarkko
>>
>> Well, your patch confuses me a lot. It looks to me like you are passing
>> the undefined `e` variable to the constructor.
>>
>> When I run flake8 on it I get following error (among others):
>>
>> F821 undefined name 'e'
>
> I don't know what flake8 is.

https://flake8.pycqa.org/en/latest/

>> What I suggested is the standard syntax:
>> https://docs.python.org/3/tutorial/errors.html
>
> It passed the Python 3 interpreter.

That is because it is technically valid syntax.

>> Did you test this? You should get an error as soon as an exception
>> occurs.
>
> Yes. Interpreter did not complain. I did not know that the language
> is broken that way that you have to exercise the code path to get
> a syntax error.

That is due to the dynamic nature of Python. You won’t get a syntax
error. You will get an exception:

NameError: name 'e' is not defined

Python has to assume that `e` might be defined at runtime. However,
style checkers will complain.

>
> /Jarkko

Cheers,
Ezra.

2020-04-14 15:22:17

by Ezra Buehler

[permalink] [raw]
Subject: Re: [PATCH] selftests/tpm2: Change exception handling to be Python 3 compatible

On 14 Apr 2020, at 09:38, Jarkko Sakkinen <[email protected]> wrote:
> OK, I'm aware about the dynamic nature but in this case it is somewhat
> counter intuitive since it is part of the exception clause. You'd except
> the Python interpreter to complain.

I agree.

> So, is Flake8 like the standard to be used?

Pretty much, yes. There is also Pylint though. Among other things, they
both check for PEP 8 (official) coding style compliance.

Cheers,
Ezra.

> On 14 Apr 2020, at 09:38, Jarkko Sakkinen <[email protected]> wrote:
>
> On Tue, Apr 14, 2020 at 07:45:33AM +0200, Ezra Buehler wrote:
>> Hi Jarkko,
>>
>> On 13 Apr 2020, at 20:04, Jarkko Sakkinen <[email protected]> wrote:
>>>
>>> On Mon, Apr 13, 2020 at 07:02:20AM +0200, Ezra Buehler wrote:
>>>> Hi Jarkko,
>>>>
>>>> On 12 Apr 2020, at 19:07, Jarkko Sakkinen <[email protected]> wrote:
>>>>>
>>>>> On Sun, Apr 12, 2020 at 05:02:27PM +0200, Ezra Buehler wrote:
>>>>>> Hi Jarkkon,
>>>>>>
>>>>>>> On 12 Apr 2020, at 16:36, Jarkko Sakkinen <[email protected]> wrote:
>>>>>>> + except ProtocolError(e):
>>>>>>
>>>>>> Should this not be
>>>>>>
>>>>>> except ProtocolError as e:
>>>>>
>>>>> Unless there is a functional difference, does it matter?
>>>>>
>>>>> /Jarkko
>>>>
>>>> Well, your patch confuses me a lot. It looks to me like you are passing
>>>> the undefined `e` variable to the constructor.
>>>>
>>>> When I run flake8 on it I get following error (among others):
>>>>
>>>> F821 undefined name 'e'
>>>
>>> I don't know what flake8 is.
>>
>> https://flake8.pycqa.org/en/latest/
>>
>>>> What I suggested is the standard syntax:
>>>> https://docs.python.org/3/tutorial/errors.html
>>>
>>> It passed the Python 3 interpreter.
>>
>> That is because it is technically valid syntax.
>>
>>>> Did you test this? You should get an error as soon as an exception
>>>> occurs.
>>>
>>> Yes. Interpreter did not complain. I did not know that the language
>>> is broken that way that you have to exercise the code path to get
>>> a syntax error.
>>
>> That is due to the dynamic nature of Python. You won’t get a syntax
>> error. You will get an exception:
>>
>> NameError: name 'e' is not defined
>>
>> Python has to assume that `e` might be defined at runtime. However,
>> style checkers will complain.
>
> OK, I'm aware about the dynamic nature but in this case it is somewhat
> counter intuitive since it is part of the exception clause. You'd except
> the Python interpreter to complain.
>
> So, is Flake8 like the standard to be used?
>
> /Jarkko

2020-04-14 22:12:31

by Jarkko Sakkinen

[permalink] [raw]
Subject: Re: [PATCH] selftests/tpm2: Change exception handling to be Python 3 compatible

On Mon, Apr 13, 2020 at 07:02:20AM +0200, Ezra Buehler wrote:
> Hi Jarkko,
>
> On 12 Apr 2020, at 19:07, Jarkko Sakkinen <[email protected]> wrote:
> >
> > On Sun, Apr 12, 2020 at 05:02:27PM +0200, Ezra Buehler wrote:
> >> Hi Jarkkon,
> >>
> >>> On 12 Apr 2020, at 16:36, Jarkko Sakkinen <[email protected]> wrote:
> >>> + except ProtocolError(e):
> >>
> >> Should this not be
> >>
> >> except ProtocolError as e:
> >
> > Unless there is a functional difference, does it matter?
> >
> > /Jarkko
>
> Well, your patch confuses me a lot. It looks to me like you are passing
> the undefined `e` variable to the constructor.
>
> When I run flake8 on it I get following error (among others):
>
> F821 undefined name 'e'

I don't know what flake8 is.

> What I suggested is the standard syntax:
> https://docs.python.org/3/tutorial/errors.html

It passed the Python 3 interpreter.

> Did you test this? You should get an error as soon as an exception
> occurs.

Yes. Interpreter did not complain. I did not know that the language
is broken that way that you have to exercise the code path to get
a syntax error.

/Jarkko

2020-04-15 11:47:16

by Jarkko Sakkinen

[permalink] [raw]
Subject: Re: [PATCH] selftests/tpm2: Change exception handling to be Python 3 compatible

On Tue, Apr 14, 2020 at 07:45:33AM +0200, Ezra Buehler wrote:
> Hi Jarkko,
>
> On 13 Apr 2020, at 20:04, Jarkko Sakkinen <[email protected]> wrote:
> >
> > On Mon, Apr 13, 2020 at 07:02:20AM +0200, Ezra Buehler wrote:
> >> Hi Jarkko,
> >>
> >> On 12 Apr 2020, at 19:07, Jarkko Sakkinen <[email protected]> wrote:
> >>>
> >>> On Sun, Apr 12, 2020 at 05:02:27PM +0200, Ezra Buehler wrote:
> >>>> Hi Jarkkon,
> >>>>
> >>>>> On 12 Apr 2020, at 16:36, Jarkko Sakkinen <[email protected]> wrote:
> >>>>> + except ProtocolError(e):
> >>>>
> >>>> Should this not be
> >>>>
> >>>> except ProtocolError as e:
> >>>
> >>> Unless there is a functional difference, does it matter?
> >>>
> >>> /Jarkko
> >>
> >> Well, your patch confuses me a lot. It looks to me like you are passing
> >> the undefined `e` variable to the constructor.
> >>
> >> When I run flake8 on it I get following error (among others):
> >>
> >> F821 undefined name 'e'
> >
> > I don't know what flake8 is.
>
> https://flake8.pycqa.org/en/latest/
>
> >> What I suggested is the standard syntax:
> >> https://docs.python.org/3/tutorial/errors.html
> >
> > It passed the Python 3 interpreter.
>
> That is because it is technically valid syntax.
>
> >> Did you test this? You should get an error as soon as an exception
> >> occurs.
> >
> > Yes. Interpreter did not complain. I did not know that the language
> > is broken that way that you have to exercise the code path to get
> > a syntax error.
>
> That is due to the dynamic nature of Python. You won’t get a syntax
> error. You will get an exception:
>
> NameError: name 'e' is not defined
>
> Python has to assume that `e` might be defined at runtime. However,
> style checkers will complain.

OK, I'm aware about the dynamic nature but in this case it is somewhat
counter intuitive since it is part of the exception clause. You'd except
the Python interpreter to complain.

So, is Flake8 like the standard to be used?

/Jarkko

2020-04-15 21:45:25

by Jarkko Sakkinen

[permalink] [raw]
Subject: Re: [PATCH] selftests/tpm2: Change exception handling to be Python 3 compatible

On Tue, Apr 14, 2020 at 01:14:11PM +0200, Ezra Buehler wrote:
> On 14 Apr 2020, at 09:38, Jarkko Sakkinen <[email protected]> wrote:
> > OK, I'm aware about the dynamic nature but in this case it is somewhat
> > counter intuitive since it is part of the exception clause. You'd except
> > the Python interpreter to complain.
>
> I agree.
>
> > So, is Flake8 like the standard to be used?
>
> Pretty much, yes. There is also Pylint though. Among other things, they
> both check for PEP 8 (official) coding style compliance.

Thank you for taking time explaining all this. I'll make sure to include
these to my process when I update my test from now on.

/Jarkko