Re: [PATCH 2/2] scripts/gdb: lx-dmesg: Use errors=replace for decoding

[Jan Kiszka]

On 2017-06-23 19:29, Leonard Crestez wrote:
> On Fri, 2017-06-23 at 18:02 +0200, Jan Kiszka wrote:
>> On 2017-06-23 16:20, Leonard Crestez wrote:
>>>
>>> It is never desirable lx-dmesg to fail on string decoding errors,
>>> not
>>> even if the log buffer is corrupt.
>>>
>>> Signed-off-by: Leonard Crestez <leonard.crestez@xxxxxxx>
>>> ---
>>>  scripts/gdb/linux/dmesg.py | 4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/scripts/gdb/linux/dmesg.py
>>> b/scripts/gdb/linux/dmesg.py
>>> index 6f8d2b2..d0cac58 100644
>>> --- a/scripts/gdb/linux/dmesg.py
>>> +++ b/scripts/gdb/linux/dmesg.py
>>> @@ -52,13 +52,13 @@ class LxDmesg(gdb.Command):
>>>                  continue
>>>  
>>>              text_len = utils.read_u16(log_buf[pos + 10:pos + 12])
>>> -            text = log_buf[pos + 16:pos + 16 + text_len].decode()
>>> +            text = log_buf[pos + 16:pos + 16 +
>>> text_len].decode(errors='replace')
>> pep8 should complain.
>>
>>>
>>>              time_stamp = utils.read_u64(log_buf[pos:pos + 8])
>>>  
>>>              for line in text.splitlines():
>>>                  gdb.write("[{time:12.6f}] {line}\n".format(
>>>                      time=time_stamp / 1000000000.0,
>>> -                    line=line))
>>> +                    line=line.encode(errors='replace')))
>> You only talk about "decoding" in the commit log, but here you encode
>> back. An short explanation why this is also needed would be nice.
>>
> Apparently .decode(errors='replace') will return an unicode string
> where invalid characters are replaced with U+FFFD REPLACEMENT
> CHARACTER. Attempting to encode that back to the default ascii encoding
> of python2 throws an error, using errors='replace' results in a '?'
> instead.
> 
> See: https://docs.python.org/2/library/codecs.html#codec-base-classes
> 
> In python3 the default encoding seems to be utf8 and errors='replace'
> is not obviously required on the encode step.
> 
> I don't actually have a gdb version compiled with python3 support and
> don't know if gdb.write always properly handles unicode in all cases.
> Perhaps it might be better to also explicitly specify 'utf8' as the
> encoding?
> 
> Linux does occasionally print unicode, for example the jffs2 driver
> shows an copyright symbol at startup. Using errors='replace' everywhere
> on python2 results in this output from lx-dmesg:
> 
> [    0.367578] jffs2: version 2.2. (NAND) ?? 2001-2006 Red Hat, Inc.
> 
> In theory if we use decode('utf8', errors='replace') and encode('utf8')
> then errors='replace' would not be required on the encode side.
> Honestly for debug code it might be preferable to do the safest
> possible thing and go 'ascii' everywhere.

I just wanted to ensure that this information is saved. If you fix the
overlong line and add something like the first paragraph to your commit
log, you can add my reviewed-by tag.

Thanks,
Jan

-- 
Siemens AG, Corporate Technology, CT RDA ITP SES-DE
Corporate Competence Center Embedded Linux