Re: [PATCH 1/2] scripts/gdb: Add version command

[Kieran Bingham <kieran.bingham@linaro.org>]

Hi Jan

On 09/01/16 16:02, Jan Kiszka wrote:
> On 2016-01-07 13:52, Kieran Bingham wrote:
>> lx-version  Report the Linux Version of the current kernel.
>>
>> Add a command to identify the version specified by the banner in the
>> debugged kernel.
>>
>> This lets the user identify the kernel of the running kernel, and will
>> let later scripts compare the banner of the attached kernel against the
>> banner in the vmlinux symbols files to verify that the files are correct.
>>
>> Signed-off-by: Kieran Bingham <kieran.bingham@linaro.org>
>> ---
>>  scripts/gdb/linux/proc.py  | 27 +++++++++++++++++++++++++++
>>  scripts/gdb/vmlinux-gdb.py |  1 +
>>  2 files changed, 28 insertions(+)
>>  create mode 100644 scripts/gdb/linux/proc.py
>>
>> diff --git a/scripts/gdb/linux/proc.py b/scripts/gdb/linux/proc.py
>> new file mode 100644
>> index 000000000000..7a2afe60416a
>> --- /dev/null
>> +++ b/scripts/gdb/linux/proc.py
>> @@ -0,0 +1,27 @@
>> +#
>> +# gdb helper commands and functions for Linux kernel debugging
>> +#
>> +#  Kernel proc information reader
>> +#
>> +# Copyright (c) 2016 Linaro Ltd
>> +#
>> +# Authors:
>> +#  Kieran Bingham <kieran.bingham@linaro.org>
>> +#
>> +# This work is licensed under the terms of the GNU GPL version 2.
>> +#
>> +
>> +import gdb
>> +
> pep8 says:
>
> scripts/gdb/linux/proc.py:16:1: E302 expected 2 blank lines, found 1

My apologies - I should have run those checks before I sent the patches.
I've added it to my checklist, to make sure I do, for any more that I send.

>
>> +class LxVersion(gdb.Command):
>> +    """ Report the Linux Version of the current kernel.
>> +        Equivalent to cat /proc/version on a running target
>> +    """
> Minor thing, but for the sake of consistency: Moving the """ into a new
> line gives an additional empty line at the end of the help output. Other
> commands, also gdb built-ins, don't do this.

And, I'll try to make sure I copy the style correctly for any follow-ups!

>
>> +    def __init__(self):
>> +        super(LxVersion, self).__init__("lx-version", gdb.COMMAND_DATA)
>> +
>> +    def invoke(self, arg, from_tty):
>> +        # linux_banner should contain a newline
>> +        gdb.write(gdb.parse_and_eval("linux_banner").string())
>> +
>> +LxVersion()
>> diff --git a/scripts/gdb/vmlinux-gdb.py b/scripts/gdb/vmlinux-gdb.py
>> index ce82bf5c3943..d5943eca19cd 100644
>> --- a/scripts/gdb/vmlinux-gdb.py
>> +++ b/scripts/gdb/vmlinux-gdb.py
>> @@ -29,3 +29,4 @@ else:
>>      import linux.tasks
>>      import linux.cpus
>>      import linux.lists
>> +    import linux.proc
>>
> Two options: I can adjust these (and the corresponding issues in patch
> 2) myself before sending out a merge request to Andrew. Or, if you have
> more in your queue, I'll wait for a potential longer v2 round. Just let
> me know.
>
> Thanks,
> Jan

If you're happy to do the fix-ups that's fine by me.
These two are simple and standalone, so I don't see any point in holding
them back.

--
Regards

Kieran