linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] Fix issue with dmesg.py and python 3.X
@ 2016-04-06  2:38 Dom Cote
  2016-04-08  3:02 ` Kieran Bingham
  0 siblings, 1 reply; 4+ messages in thread
From: Dom Cote @ 2016-04-06  2:38 UTC (permalink / raw)
  To: kieran.bingham, jan.kiszka; +Cc: linux-kernel

When using GDB built with python 2.7,

Inferior.read_memory (address, length)

returns a buffer object. When using GDB built with python 3.X,
it returns a memoryview object, which cannot be added to another
memoryview object.

Replace the addition (+) of 2 python memoryview objects
with the addition of 2 'bytes' objects.

Create a read_memoryview() function that always return a memoryview
object.

Change the read_u16 function so it doesn't need to use ord()
anymore.

Tested with Python 3.4 and gdb 7.7

Signed-off-by: Dom Cote <buzdelabuz2@gmail.com>
---
 scripts/gdb/linux/dmesg.py | 9 +++++----
 scripts/gdb/linux/utils.py | 9 +++++++--
 2 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/scripts/gdb/linux/dmesg.py b/scripts/gdb/linux/dmesg.py
index 927d0d2a3145..96f4732157d8 100644
--- a/scripts/gdb/linux/dmesg.py
+++ b/scripts/gdb/linux/dmesg.py
@@ -33,11 +33,12 @@ class LxDmesg(gdb.Command):
         if log_first_idx < log_next_idx:
             log_buf_2nd_half = -1
             length = log_next_idx - log_first_idx
-            log_buf = inf.read_memory(start, length)
+            log_buf = utils.read_memoryview(inf, start, length).tobytes()
         else:
             log_buf_2nd_half = log_buf_len - log_first_idx
-            log_buf = inf.read_memory(start, log_buf_2nd_half) + \
-                inf.read_memory(log_buf_addr, log_next_idx)
+            a = utils.read_memoryview(inf, start, log_buf_2nd_half)
+            b = utils.read_memoryview(inf, log_buf_addr, log_next_idx)
+            log_buf = a.tobytes() + b.tobytes()
 
         pos = 0
         while pos < log_buf.__len__():
@@ -53,7 +54,7 @@ class LxDmesg(gdb.Command):
             text = log_buf[pos + 16:pos + 16 + text_len]
             time_stamp = utils.read_u64(log_buf[pos:pos + 8])
 
-            for line in memoryview(text).tobytes().splitlines():
+            for line in text.splitlines():
                 gdb.write("[{time:12.6f}] {line}\n".format(
                     time=time_stamp / 1000000000.0,
                     line=line))
diff --git a/scripts/gdb/linux/utils.py b/scripts/gdb/linux/utils.py
index 0893b326a28b..c2b779e7bd26 100644
--- a/scripts/gdb/linux/utils.py
+++ b/scripts/gdb/linux/utils.py
@@ -87,11 +87,16 @@ def get_target_endianness():
     return target_endianness
 
 
+# Compat between GDB built with python 2.7 vs 3.X
+def read_memoryview(inf, start, length):
+    return memoryview(inf.read_memory(start, length))
+
+
 def read_u16(buffer):
     if get_target_endianness() == LITTLE_ENDIAN:
-        return ord(buffer[0]) + (ord(buffer[1]) << 8)
+        return buffer[0] + (buffer[1] << 8)
     else:
-        return ord(buffer[1]) + (ord(buffer[0]) << 8)
+        return buffer[1] + (buffer[0] << 8)
 
 
 def read_u32(buffer):
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] Fix issue with dmesg.py and python 3.X
  2016-04-06  2:38 [PATCH] Fix issue with dmesg.py and python 3.X Dom Cote
@ 2016-04-08  3:02 ` Kieran Bingham
       [not found]   ` <CA+Bh5YxOw_NcrMk7MhN22nX35kc2bFgMrHZo_aFh7gk4o-5cmg@mail.gmail.com>
  0 siblings, 1 reply; 4+ messages in thread
From: Kieran Bingham @ 2016-04-08  3:02 UTC (permalink / raw)
  To: Dom Cote, jan.kiszka; +Cc: linux-kernel

Hi Dom,

I've just tested your patch quickly, and it generates an error on Python 2.7


On 05/04/16 19:38, Dom Cote wrote:
> When using GDB built with python 2.7,
> 
> Inferior.read_memory (address, length)
> 
> returns a buffer object. When using GDB built with python 3.X,
> it returns a memoryview object, which cannot be added to another
> memoryview object.
> 
> Replace the addition (+) of 2 python memoryview objects
> with the addition of 2 'bytes' objects.
> 
> Create a read_memoryview() function that always return a memoryview
> object.

The change to memoryview appears to work - but I don't think it needs to
be indirected by a function definition?


> Change the read_u16 function so it doesn't need to use ord()
> anymore.

This change is separate to the memoryview object, and should be in it's
own patch. One patch should fix one thing independently.

For example, the change to memoryview object appears to be functional,
and the read_u16 is not. If these changes are in separate patches, then
working changes can be accepted sooner, and tested easier.


> Tested with Python 3.4 and gdb 7.7
> 
> Signed-off-by: Dom Cote <buzdelabuz2@gmail.com>
> ---
>  scripts/gdb/linux/dmesg.py | 9 +++++----
>  scripts/gdb/linux/utils.py | 9 +++++++--
>  2 files changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/scripts/gdb/linux/dmesg.py b/scripts/gdb/linux/dmesg.py
> index 927d0d2a3145..96f4732157d8 100644
> --- a/scripts/gdb/linux/dmesg.py
> +++ b/scripts/gdb/linux/dmesg.py
> @@ -33,11 +33,12 @@ class LxDmesg(gdb.Command):
>          if log_first_idx < log_next_idx:
>              log_buf_2nd_half = -1
>              length = log_next_idx - log_first_idx
> -            log_buf = inf.read_memory(start, length)
> +            log_buf = utils.read_memoryview(inf, start, length).tobytes()

This looks like it could just call memoryview() directly ...

>          else:
>              log_buf_2nd_half = log_buf_len - log_first_idx
> -            log_buf = inf.read_memory(start, log_buf_2nd_half) + \
> -                inf.read_memory(log_buf_addr, log_next_idx)
> +            a = utils.read_memoryview(inf, start, log_buf_2nd_half)
> +            b = utils.read_memoryview(inf, log_buf_addr, log_next_idx)

Likewise here I presume ...

> +            log_buf = a.tobytes() + b.tobytes()
>  
>          pos = 0
>          while pos < log_buf.__len__():
> @@ -53,7 +54,7 @@ class LxDmesg(gdb.Command):
>              text = log_buf[pos + 16:pos + 16 + text_len]
>              time_stamp = utils.read_u64(log_buf[pos:pos + 8])
>  
> -            for line in memoryview(text).tobytes().splitlines():
> +            for line in text.splitlines():

This looks like a separate change, not related to the bug fix?
If this is an improvement, rather than a fix - it should probably also
have it's own patch. (at first glance it looks like an improvement :D )

I just haven't seen yet if it depends on the other change.

>                  gdb.write("[{time:12.6f}] {line}\n".format(
>                      time=time_stamp / 1000000000.0,
>                      line=line))
> diff --git a/scripts/gdb/linux/utils.py b/scripts/gdb/linux/utils.py
> index 0893b326a28b..c2b779e7bd26 100644
> --- a/scripts/gdb/linux/utils.py
> +++ b/scripts/gdb/linux/utils.py
> @@ -87,11 +87,16 @@ def get_target_endianness():
>      return target_endianness
>  
>  
> +# Compat between GDB built with python 2.7 vs 3.X
> +def read_memoryview(inf, start, length):
> +    return memoryview(inf.read_memory(start, length))

This simply always returns a memoryview object, so why not just change
the respective lines, which you have already had to modify to call/use
memoryview directly?

> +
> +
>  def read_u16(buffer):
>      if get_target_endianness() == LITTLE_ENDIAN:
> -        return ord(buffer[0]) + (ord(buffer[1]) << 8)
> +        return buffer[0] + (buffer[1] << 8)
>      else:
> -        return ord(buffer[1]) + (ord(buffer[0]) << 8)
> +        return buffer[1] + (buffer[0] << 8)

This breaks for me, but returning these lines to use ord() shows that
the memoryview changes appear to work.

What was the need for changing these lines? Does it cause a break on 3.x?

This causes the following error on 2.7 for me:

(gdb) lx-dmesg
Traceback (most recent call last):
File
"/home/linuxembedded/linaro/lkd/openst-lkd/objects/arm/qemu-arm/linux/scripts/gdb/linux/dmesg.py",
line 45, in invoke
length = utils.read_u16(log_buf[pos + 8:pos + 10])
File
"/home/linuxembedded/linaro/lkd/openst-lkd/objects/arm/qemu-arm/linux/scripts/gdb/linux/utils.py",
line 97, in read_u16
return buffer[0] + (buffer[1] << 8)
TypeError: unsupported operand type(s) for <<: 'str' and 'int'
Error occurred in Python command: unsupported operand type(s) for <<:
'str' and 'int'

>  
>  
>  def read_u32(buffer):
> 


--
Regards

Kieran

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] Fix issue with dmesg.py and python 3.X
       [not found]   ` <CA+Bh5YxOw_NcrMk7MhN22nX35kc2bFgMrHZo_aFh7gk4o-5cmg@mail.gmail.com>
@ 2016-04-29 15:03     ` Kieran Bingham
       [not found]       ` <CA+Bh5YwYRKVywgKY5Jf+Hy33Y9mTCL+_LdUKBuXuXHvr6bcZxw@mail.gmail.com>
  0 siblings, 1 reply; 4+ messages in thread
From: Kieran Bingham @ 2016-04-29 15:03 UTC (permalink / raw)
  To: Dom Cote; +Cc: J. Kiszka, linux-kernel

Hi Dom,

On 08/04/16 04:33, Dom Cote wrote:
> Hi Kieran,
> 
> Thanks for the feedback. 
> 
> I am going to need to build a version of gdb with python 2.7 to see what
> the differences are, and try to abstract them in the best possible way.
> 
> Then I will send a new patch and incorporate what you just highlighted
> in it.

I was just wondering if you had chance to look at any of this ? (I was
looking at it again myself)

> Regards
> 
> Dom
> 
> 
> On Thu, Apr 7, 2016 at 11:02 PM, Kieran Bingham
> <kieran.bingham@linaro.org <mailto:kieran.bingham@linaro.org>> wrote:
> 
>     Hi Dom,
> 
>     I've just tested your patch quickly, and it generates an error on
>     Python 2.7
> 
> 
>     On 05/04/16 19:38, Dom Cote wrote:
>     > When using GDB built with python 2.7,
>     >
>     > Inferior.read_memory (address, length)
>     >
>     > returns a buffer object. When using GDB built with python 3.X,
>     > it returns a memoryview object, which cannot be added to another
>     > memoryview object.
>     >
>     > Replace the addition (+) of 2 python memoryview objects
>     > with the addition of 2 'bytes' objects.
>     >
>     > Create a read_memoryview() function that always return a memoryview
>     > object.
> 
>     The change to memoryview appears to work - but I don't think it needs to
>     be indirected by a function definition?
> 
> 
>     > Change the read_u16 function so it doesn't need to use ord()
>     > anymore.
> 
>     This change is separate to the memoryview object, and should be in it's
>     own patch. One patch should fix one thing independently.
> 
>     For example, the change to memoryview object appears to be functional,
>     and the read_u16 is not. If these changes are in separate patches, then
>     working changes can be accepted sooner, and tested easier.
> 
> 
>     > Tested with Python 3.4 and gdb 7.7
>     >
>     > Signed-off-by: Dom Cote <buzdelabuz2@gmail.com <mailto:buzdelabuz2@gmail.com>>
>     > ---
>     >  scripts/gdb/linux/dmesg.py | 9 +++++----
>     >  scripts/gdb/linux/utils.py | 9 +++++++--
>     >  2 files changed, 12 insertions(+), 6 deletions(-)
>     >
>     > diff --git a/scripts/gdb/linux/dmesg.py b/scripts/gdb/linux/dmesg.py
>     > index 927d0d2a3145..96f4732157d8 100644
>     > --- a/scripts/gdb/linux/dmesg.py
>     > +++ b/scripts/gdb/linux/dmesg.py
>     > @@ -33,11 +33,12 @@ class LxDmesg(gdb.Command):
>     >          if log_first_idx < log_next_idx:
>     >              log_buf_2nd_half = -1
>     >              length = log_next_idx - log_first_idx
>     > -            log_buf = inf.read_memory(start, length)
>     > +            log_buf = utils.read_memoryview(inf, start, length).tobytes()
> 
>     This looks like it could just call memoryview() directly ...

I just looked at the line sizes:

- log_buf = inf.read_memory(start, length)
+ log_buf = utils.read_memoryview(inf, start, length).tobytes()
+ log_buf = memoryview(inf.read_memory(start, length)).tobytes()

There's not a lot in it, and saving one char is probably not enough
reason to add a function call.

> 
>     >          else:
>     >              log_buf_2nd_half = log_buf_len - log_first_idx
>     > -            log_buf = inf.read_memory(start, log_buf_2nd_half) + \
>     > -                inf.read_memory(log_buf_addr, log_next_idx)
>     > +            a = utils.read_memoryview(inf, start, log_buf_2nd_half)
>     > +            b = utils.read_memoryview(inf, log_buf_addr, log_next_idx)
> 
>     Likewise here I presume ...
> 
>     > +            log_buf = a.tobytes() + b.tobytes()
>     >
>     >          pos = 0
>     >          while pos < log_buf.__len__():
>     > @@ -53,7 +54,7 @@ class LxDmesg(gdb.Command):
>     >              text = log_buf[pos + 16:pos + 16 + text_len]
>     >              time_stamp = utils.read_u64(log_buf[pos:pos + 8])
>     >
>     > -            for line in memoryview(text).tobytes().splitlines():
>     > +            for line in text.splitlines():
> 
>     This looks like a separate change, not related to the bug fix?
>     If this is an improvement, rather than a fix - it should probably also
>     have it's own patch. (at first glance it looks like an improvement :D )
> 
>     I just haven't seen yet if it depends on the other change.
> 
>     >                  gdb.write("[{time:12.6f}] {line}\n".format(
>     >                      time=time_stamp / 1000000000.0,
>     >                      line=line))
>     > diff --git a/scripts/gdb/linux/utils.py b/scripts/gdb/linux/utils.py
>     > index 0893b326a28b..c2b779e7bd26 100644
>     > --- a/scripts/gdb/linux/utils.py
>     > +++ b/scripts/gdb/linux/utils.py
>     > @@ -87,11 +87,16 @@ def get_target_endianness():
>     >      return target_endianness
>     >
>     >
>     > +# Compat between GDB built with python 2.7 vs 3.X
>     > +def read_memoryview(inf, start, length):
>     > +    return memoryview(inf.read_memory(start, length))
> 
>     This simply always returns a memoryview object, so why not just change
>     the respective lines, which you have already had to modify to call/use
>     memoryview directly?
> 
>     > +
>     > +
>     >  def read_u16(buffer):
>     >      if get_target_endianness() == LITTLE_ENDIAN:
>     > -        return ord(buffer[0]) + (ord(buffer[1]) << 8)
>     > +        return buffer[0] + (buffer[1] << 8)
>     >      else:
>     > -        return ord(buffer[1]) + (ord(buffer[0]) << 8)
>     > +        return buffer[1] + (buffer[0] << 8)
> 
>     This breaks for me, but returning these lines to use ord() shows that
>     the memoryview changes appear to work.
> 
>     What was the need for changing these lines? Does it cause a break on
>     3.x?
> 
>     This causes the following error on 2.7 for me:
> 
>     (gdb) lx-dmesg
>     Traceback (most recent call last):
>     File
>     "/home/linuxembedded/linaro/lkd/openst-lkd/objects/arm/qemu-arm/linux/scripts/gdb/linux/dmesg.py",
>     line 45, in invoke
>     length = utils.read_u16(log_buf[pos + 8:pos + 10])
>     File
>     "/home/linuxembedded/linaro/lkd/openst-lkd/objects/arm/qemu-arm/linux/scripts/gdb/linux/utils.py",
>     line 97, in read_u16
>     return buffer[0] + (buffer[1] << 8)
>     TypeError: unsupported operand type(s) for <<: 'str' and 'int'
>     Error occurred in Python command: unsupported operand type(s) for <<:
>     'str' and 'int'

I guess we're going to have to do something along the lines of
  PY2 = (sys.version_info[0] == 2)

  if PY2:
     # Do Py2 compatible code
  else:
     # Do Py3+ stuff

to support both targets. (Ugh...) But this isn't the first time I've
been asked if we support both Python 2 and Python 3 so I guess it will
become an issue.


Anyway, let me know if there's anything I can do to help / test.

-- 
Regards

Kieran Bingham

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] Fix issue with dmesg.py and python 3.X
       [not found]       ` <CA+Bh5YwYRKVywgKY5Jf+Hy33Y9mTCL+_LdUKBuXuXHvr6bcZxw@mail.gmail.com>
@ 2016-05-03  8:34         ` Kieran Bingham
  0 siblings, 0 replies; 4+ messages in thread
From: Kieran Bingham @ 2016-05-03  8:34 UTC (permalink / raw)
  To: Dom Cote; +Cc: J. Kiszka, linux-kernel

On 29/04/16 20:00, Dom Cote wrote:
> Hi Kieran,

Hi Dom, sorry for the delay, we had a 3 day weekend over here :)

> 
> Thanks for pinging me on this. 
> 
> I had to rebuild a version of GDB with python 2.7 (mine system has 3.X).
> I had to dig into the configure.ac <http://configure.ac> script to find
> out what I needed to do to get the gdb build to pick up the python 2.7
> build. 
> But I got it to work now.

Excellent!


> Back to the issue:
> 
> With python 2.7, we end up with log_buf elements being of type 'str'
> (still), where as with python 3.X, they are of type 'byte'.
> 
> So we could solve this issue by checking the type of the arguments
> inside read_u16(), in utils.py (see below).
> 
> It would be more generic than checking for versions of python.

Actually I really like this (in small quantities). Instead of checking
an arbitrary version - it is checking the article it actually needs to
adapt for.

I fear the day that we may have conditionals littered everywhere due to
Python API change, - but actually we seem to be ok for the most part,
and that day is not today.

> I tested this both on 2.7 and 3.X (after having reproduced the problem
> you reported with my previous patch), and it works on both.
> 
> The changes made to dmesg.py would stay as they are, and it wouldn't
> have version dependent code in it.

Perfect. Generic dmesg :) with the read helpers doing the help to adapt!
Sounds like a good approach to me.

Will this have an effect on read_{u32,u64}? Or will they work as
expected (they call down to read_u16, so I think they will be ok)

> What do you think ? Should I submit another patch  using that approach ?

Yes, please do!

If you can, separate out the functional changes into their own commits,
it makes the integration process much easier. (changes to utils in one
patch, any improvements to dmesg in another)

Thanks

Kieran

> Regards
> 
> Dom
> 
> --------------
> 
> def read_u16(buffer):
>     value = [0, 0]
> 
>     if type(buffer[0]) is str:
>         value[0] = ord(buffer[0])
>         value[1] = ord(buffer[1])
>     else:
>         value[0] = buffer[0]
>         value[1] = buffer[1]
> 
>     if get_target_endianness() == LITTLE_ENDIAN:
>         return value[0] + (value[1] << 8)
>     else:
>         return value[1] + (value[0] << 8)
> 
> 
> 
> On Fri, Apr 29, 2016 at 11:03 AM, Kieran Bingham <kieran@bingham.xyz
> <mailto:kieran@bingham.xyz>> wrote:
> 
>     Hi Dom,
> 
>     On 08/04/16 04:33, Dom Cote wrote:
>     > Hi Kieran,
>     >
>     > Thanks for the feedback.
>     >
>     > I am going to need to build a version of gdb with python 2.7 to see what
>     > the differences are, and try to abstract them in the best possible way.
>     >
>     > Then I will send a new patch and incorporate what you just highlighted
>     > in it.
> 
>     I was just wondering if you had chance to look at any of this ? (I was
>     looking at it again myself)
> 
>     > Regards
>     >
>     > Dom
>     >
>     >
>     > On Thu, Apr 7, 2016 at 11:02 PM, Kieran Bingham
>     > <kieran.bingham@linaro.org <mailto:kieran.bingham@linaro.org>
>     <mailto:kieran.bingham@linaro.org
>     <mailto:kieran.bingham@linaro.org>>> wrote:
>     >
>     >     Hi Dom,
>     >
>     >     I've just tested your patch quickly, and it generates an error on
>     >     Python 2.7
>     >
>     >
>     >     On 05/04/16 19:38, Dom Cote wrote:
>     >     > When using GDB built with python 2.7,
>     >     >
>     >     > Inferior.read_memory (address, length)
>     >     >
>     >     > returns a buffer object. When using GDB built with python 3.X,
>     >     > it returns a memoryview object, which cannot be added to another
>     >     > memoryview object.
>     >     >
>     >     > Replace the addition (+) of 2 python memoryview objects
>     >     > with the addition of 2 'bytes' objects.
>     >     >
>     >     > Create a read_memoryview() function that always return a
>     memoryview
>     >     > object.
>     >
>     >     The change to memoryview appears to work - but I don't think
>     it needs to
>     >     be indirected by a function definition?
>     >
>     >
>     >     > Change the read_u16 function so it doesn't need to use ord()
>     >     > anymore.
>     >
>     >     This change is separate to the memoryview object, and should
>     be in it's
>     >     own patch. One patch should fix one thing independently.
>     >
>     >     For example, the change to memoryview object appears to be
>     functional,
>     >     and the read_u16 is not. If these changes are in separate
>     patches, then
>     >     working changes can be accepted sooner, and tested easier.
>     >
>     >
>     >     > Tested with Python 3.4 and gdb 7.7
>     >     >
>     >     > Signed-off-by: Dom Cote <buzdelabuz2@gmail.com
>     <mailto:buzdelabuz2@gmail.com> <mailto:buzdelabuz2@gmail.com
>     <mailto:buzdelabuz2@gmail.com>>>
>     >     > ---
>     >     >  scripts/gdb/linux/dmesg.py | 9 +++++----
>     >     >  scripts/gdb/linux/utils.py | 9 +++++++--
>     >     >  2 files changed, 12 insertions(+), 6 deletions(-)
>     >     >
>     >     > diff --git a/scripts/gdb/linux/dmesg.py b/scripts/gdb/linux/dmesg.py
>     >     > index 927d0d2a3145..96f4732157d8 100644
>     >     > --- a/scripts/gdb/linux/dmesg.py
>     >     > +++ b/scripts/gdb/linux/dmesg.py
>     >     > @@ -33,11 +33,12 @@ class LxDmesg(gdb.Command):
>     >     >          if log_first_idx < log_next_idx:
>     >     >              log_buf_2nd_half = -1
>     >     >              length = log_next_idx - log_first_idx
>     >     > -            log_buf = inf.read_memory(start, length)
>     >     > +            log_buf = utils.read_memoryview(inf, start, length).tobytes()
>     >
>     >     This looks like it could just call memoryview() directly ...
> 
>     I just looked at the line sizes:
> 
>     - log_buf = inf.read_memory(start, length)
>     + log_buf = utils.read_memoryview(inf, start, length).tobytes()
>     + log_buf = memoryview(inf.read_memory(start, length)).tobytes()
> 
>     There's not a lot in it, and saving one char is probably not enough
>     reason to add a function call.
> 
>     >
>     >     >          else:
>     >     >              log_buf_2nd_half = log_buf_len - log_first_idx
>     >     > -            log_buf = inf.read_memory(start,
>     log_buf_2nd_half) + \
>     >     > -                inf.read_memory(log_buf_addr, log_next_idx)
>     >     > +            a = utils.read_memoryview(inf, start,
>     log_buf_2nd_half)
>     >     > +            b = utils.read_memoryview(inf, log_buf_addr,
>     log_next_idx)
>     >
>     >     Likewise here I presume ...
>     >
>     >     > +            log_buf = a.tobytes() + b.tobytes()
>     >     >
>     >     >          pos = 0
>     >     >          while pos < log_buf.__len__():
>     >     > @@ -53,7 +54,7 @@ class LxDmesg(gdb.Command):
>     >     >              text = log_buf[pos + 16:pos + 16 + text_len]
>     >     >              time_stamp = utils.read_u64(log_buf[pos:pos + 8])
>     >     >
>     >     > -            for line in
>     memoryview(text).tobytes().splitlines():
>     >     > +            for line in text.splitlines():
>     >
>     >     This looks like a separate change, not related to the bug fix?
>     >     If this is an improvement, rather than a fix - it should
>     probably also
>     >     have it's own patch. (at first glance it looks like an
>     improvement :D )
>     >
>     >     I just haven't seen yet if it depends on the other change.
>     >
>     >     >                  gdb.write("[{time:12.6f}] {line}\n".format(
>     >     >                      time=time_stamp / 1000000000.0,
>     >     >                      line=line))
>     >     > diff --git a/scripts/gdb/linux/utils.py
>     b/scripts/gdb/linux/utils.py
>     >     > index 0893b326a28b..c2b779e7bd26 100644
>     >     > --- a/scripts/gdb/linux/utils.py
>     >     > +++ b/scripts/gdb/linux/utils.py
>     >     > @@ -87,11 +87,16 @@ def get_target_endianness():
>     >     >      return target_endianness
>     >     >
>     >     >
>     >     > +# Compat between GDB built with python 2.7 vs 3.X
>     >     > +def read_memoryview(inf, start, length):
>     >     > +    return memoryview(inf.read_memory(start, length))
>     >
>     >     This simply always returns a memoryview object, so why not
>     just change
>     >     the respective lines, which you have already had to modify to
>     call/use
>     >     memoryview directly?
>     >
>     >     > +
>     >     > +
>     >     >  def read_u16(buffer):
>     >     >      if get_target_endianness() == LITTLE_ENDIAN:
>     >     > -        return ord(buffer[0]) + (ord(buffer[1]) << 8)
>     >     > +        return buffer[0] + (buffer[1] << 8)
>     >     >      else:
>     >     > -        return ord(buffer[1]) + (ord(buffer[0]) << 8)
>     >     > +        return buffer[1] + (buffer[0] << 8)
>     >
>     >     This breaks for me, but returning these lines to use ord()
>     shows that
>     >     the memoryview changes appear to work.
>     >
>     >     What was the need for changing these lines? Does it cause a
>     break on
>     >     3.x?
>     >
>     >     This causes the following error on 2.7 for me:
>     >
>     >     (gdb) lx-dmesg
>     >     Traceback (most recent call last):
>     >     File
>     >   
>      "/home/linuxembedded/linaro/lkd/openst-lkd/objects/arm/qemu-arm/linux/scripts/gdb/linux/dmesg.py",
>     >     line 45, in invoke
>     >     length = utils.read_u16(log_buf[pos + 8:pos + 10])
>     >     File
>     >   
>      "/home/linuxembedded/linaro/lkd/openst-lkd/objects/arm/qemu-arm/linux/scripts/gdb/linux/utils.py",
>     >     line 97, in read_u16
>     >     return buffer[0] + (buffer[1] << 8)
>     >     TypeError: unsupported operand type(s) for <<: 'str' and 'int'
>     >     Error occurred in Python command: unsupported operand type(s)
>     for <<:
>     >     'str' and 'int'
> 
>     I guess we're going to have to do something along the lines of
>       PY2 = (sys.version_info[0] == 2)
> 
>       if PY2:
>          # Do Py2 compatible code
>       else:
>          # Do Py3+ stuff
> 
>     to support both targets. (Ugh...) But this isn't the first time I've
>     been asked if we support both Python 2 and Python 3 so I guess it will
>     become an issue.
> 
> 
>     Anyway, let me know if there's anything I can do to help / test.
> 
>     --
>     Regards
> 
>     Kieran Bingham
> 
> 

-- 
Regards

Kieran Bingham

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2016-05-03  8:34 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-06  2:38 [PATCH] Fix issue with dmesg.py and python 3.X Dom Cote
2016-04-08  3:02 ` Kieran Bingham
     [not found]   ` <CA+Bh5YxOw_NcrMk7MhN22nX35kc2bFgMrHZo_aFh7gk4o-5cmg@mail.gmail.com>
2016-04-29 15:03     ` Kieran Bingham
     [not found]       ` <CA+Bh5YwYRKVywgKY5Jf+Hy33Y9mTCL+_LdUKBuXuXHvr6bcZxw@mail.gmail.com>
2016-05-03  8:34         ` Kieran Bingham

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).