diff mbox series

[v20210701,06/40] tools: fix Python3.4 TypeError in format string

Message ID 20210701095635.15648-7-olaf@aepfle.de (mailing list archive)
State Superseded
Headers show
Series leftover from 2020 | expand

Commit Message

Olaf Hering July 1, 2021, 9:56 a.m. UTC
Using the first element of a tuple for a format specifier fails with
python3.4 as included in SLE12:
    b = b"string/%x" % (i, )
TypeError: unsupported operand type(s) for %: 'bytes' and 'tuple'

It happens to work with python 2.7 and 3.6.
Use a syntax that is handled by all three variants.

Signed-off-by: Olaf Hering <olaf@aepfle.de>
---
 tools/python/scripts/convert-legacy-stream | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

Marek Marczykowski-Górecki July 2, 2021, 4:19 p.m. UTC | #1
On Thu, Jul 01, 2021 at 11:56:01AM +0200, Olaf Hering wrote:
> Using the first element of a tuple for a format specifier fails with
> python3.4 as included in SLE12:
>     b = b"string/%x" % (i, )
> TypeError: unsupported operand type(s) for %: 'bytes' and 'tuple'
> 
> It happens to work with python 2.7 and 3.6.
> Use a syntax that is handled by all three variants.
> 
> Signed-off-by: Olaf Hering <olaf@aepfle.de>
> ---
>  tools/python/scripts/convert-legacy-stream | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/python/scripts/convert-legacy-stream b/tools/python/scripts/convert-legacy-stream
> index 9003ac4f6d..235b922ff5 100755
> --- a/tools/python/scripts/convert-legacy-stream
> +++ b/tools/python/scripts/convert-legacy-stream
> @@ -347,9 +347,9 @@ def read_libxl_toolstack(vm, data):
>          if nil != 0:
>              raise StreamError("physmap name not NUL terminated")
>  
> -        root = b"physmap/%x" % (phys, )
> -        kv = [root + b"/start_addr", b"%x" % (start, ),
> -              root + b"/size",       b"%x" % (size, ),
> +        root = bytes(("physmap/%x" % phys).encode('utf-8'))
> +        kv = [root + b"/start_addr", bytes(("%x" % start).encode('utf-8')),
> +              root + b"/size",       bytes(("%x" % size).encode('utf-8')),

Why bytes()? Encode does already return bytes type.

>                root + b"/name",       name]
>  
>          for key, val in zip(kv[0::2], kv[1::2]):
Andrew Cooper July 2, 2021, 4:39 p.m. UTC | #2
On 02/07/2021 17:19, Marek Marczykowski-Górecki wrote:
> On Thu, Jul 01, 2021 at 11:56:01AM +0200, Olaf Hering wrote:
>> Using the first element of a tuple for a format specifier fails with
>> python3.4 as included in SLE12:
>>     b = b"string/%x" % (i, )
>> TypeError: unsupported operand type(s) for %: 'bytes' and 'tuple'
>>
>> It happens to work with python 2.7 and 3.6.
>> Use a syntax that is handled by all three variants.
>>
>> Signed-off-by: Olaf Hering <olaf@aepfle.de>
>> ---
>>  tools/python/scripts/convert-legacy-stream | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/tools/python/scripts/convert-legacy-stream b/tools/python/scripts/convert-legacy-stream
>> index 9003ac4f6d..235b922ff5 100755
>> --- a/tools/python/scripts/convert-legacy-stream
>> +++ b/tools/python/scripts/convert-legacy-stream
>> @@ -347,9 +347,9 @@ def read_libxl_toolstack(vm, data):
>>          if nil != 0:
>>              raise StreamError("physmap name not NUL terminated")
>>  
>> -        root = b"physmap/%x" % (phys, )
>> -        kv = [root + b"/start_addr", b"%x" % (start, ),
>> -              root + b"/size",       b"%x" % (size, ),
>> +        root = bytes(("physmap/%x" % phys).encode('utf-8'))
>> +        kv = [root + b"/start_addr", bytes(("%x" % start).encode('utf-8')),
>> +              root + b"/size",       bytes(("%x" % size).encode('utf-8')),
> Why bytes()? Encode does already return bytes type.

Yes - I've just tried this out on various version of python (including
https://www.onlinegdb.com/online_python_interpreter which is the only
place I can find Python 3.4 easily available)

.encode() does return bytes (Py3) or str (Py2) so doesn't need the
surrounding bytes().

However, the % (phys, ) with the trailing comma is deliberate to work
around a common python error, so wants to remain if you're keeping the
%-formatting.

~Andrew
Olaf Hering July 5, 2021, 8:07 a.m. UTC | #3
Am Fri, 2 Jul 2021 18:19:39 +0200
schrieb Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>:

> Why bytes()? Encode does already return bytes type.

You are right, this works as well:
  i = 123
  b = ("str/%x" % (i, )).encode('utf-8')

Any preference regarding the "encoding"? I picked UTF8, but 'ascii' might be more correct in this context. In practice it may not matter.


Olaf
Olaf Hering July 5, 2021, 8:18 a.m. UTC | #4
Am Fri, 2 Jul 2021 17:39:54 +0100
schrieb Andrew Cooper <andrew.cooper3@citrix.com>:

> However, the % (phys, ) with the trailing comma is deliberate to work
> around a common python error, so wants to remain if you're keeping the
> %-formatting.

What error is that?

Olaf
Andrew Cooper July 5, 2021, 9:47 a.m. UTC | #5
On 05/07/2021 09:18, Olaf Hering wrote:
> Am Fri, 2 Jul 2021 17:39:54 +0100
> schrieb Andrew Cooper <andrew.cooper3@citrix.com>:
>
>> However, the % (phys, ) with the trailing comma is deliberate to work
>> around a common python error, so wants to remain if you're keeping the
>> %-formatting.
> What error is that?

>>> def p1(arg):
...     print("%s" % arg)
>>> def p2(arg):
...     print("%s" % (arg, ))

>>> p1("foo")
foo
>>> p2("foo")
foo

>>> p1(("foo", "bar"))
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "<stdin>", line 2, in p1
TypeError: not all arguments converted during string formatting

>>> p2(("foo", "bar"))
('foo', 'bar')


The % operator has some type ambiguity with how it works.  (foo, )
forces arg to be a 1-tuple as far as formatting is concerned.

~Andrew
Andrew Cooper July 5, 2021, 10:10 a.m. UTC | #6
On 05/07/2021 09:07, Olaf Hering wrote:
> Am Fri, 2 Jul 2021 18:19:39 +0200
> schrieb Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>:
>
>> Why bytes()? Encode does already return bytes type.
> You are right, this works as well:
>   i = 123
>   b = ("str/%x" % (i, )).encode('utf-8')
>
> Any preference regarding the "encoding"? I picked UTF8, but 'ascii' might be more correct in this context. In practice it may not matter.

I suspect you're right and it doesn't matter, but ascii feels like a
safer option.

~Andrew
diff mbox series

Patch

diff --git a/tools/python/scripts/convert-legacy-stream b/tools/python/scripts/convert-legacy-stream
index 9003ac4f6d..235b922ff5 100755
--- a/tools/python/scripts/convert-legacy-stream
+++ b/tools/python/scripts/convert-legacy-stream
@@ -347,9 +347,9 @@  def read_libxl_toolstack(vm, data):
         if nil != 0:
             raise StreamError("physmap name not NUL terminated")
 
-        root = b"physmap/%x" % (phys, )
-        kv = [root + b"/start_addr", b"%x" % (start, ),
-              root + b"/size",       b"%x" % (size, ),
+        root = bytes(("physmap/%x" % phys).encode('utf-8'))
+        kv = [root + b"/start_addr", bytes(("%x" % start).encode('utf-8')),
+              root + b"/size",       bytes(("%x" % size).encode('utf-8')),
               root + b"/name",       name]
 
         for key, val in zip(kv[0::2], kv[1::2]):