Re: [PATCHv3 02/13] scripts/gdb: Provide kernel list item generators
From: Kieran Bingham
Date: Tue Mar 08 2016 - 02:55:39 EST
On 08/03/16 10:47, Jeff Mahoney wrote:
> On 3/3/16 6:40 AM, Kieran Bingham wrote:
>> Facilitate linked-list items by providing a generator to return
>> the dereferenced, and type-cast objects from a kernel linked list
>>
>> CC: Jeff Mahoney <jeffm@xxxxxxxx>
>>
>> Signed-off-by: Kieran Bingham <kieran.bingham@xxxxxxxxxx>
>> ---
>> Changes since v1:
>> * items function removed, and replaced with Jeff Mahoney's cleaner
>> implementations of list_for_each, and list_for_each_entry
>> ---
>> scripts/gdb/linux/lists.py | 20 ++++++++++++++++++++
>> 1 file changed, 20 insertions(+)
>>
>> diff --git a/scripts/gdb/linux/lists.py b/scripts/gdb/linux/lists.py
>> index 3a3775bc162b..9f4503738e26 100644
>> --- a/scripts/gdb/linux/lists.py
>> +++ b/scripts/gdb/linux/lists.py
>> @@ -18,6 +18,26 @@ from linux import utils
>> list_head = utils.CachedType("struct list_head")
>>
>>
>> +def list_for_each(head):
>> + if head.type == list_head.get_type().pointer():
>> + head = head.dereference()
>> + elif head.type != list_head.get_type():
>> + raise gdb.GdbError("Must be struct list_head not %s" % list_head.type)
>
> Shouldn't this be % head.type?
Ahh yes, good spot thanks!
>
>> +
>> + node = head['next'].dereference()
>> + while node.address != head.address:
>> + yield node.address
>> + node = node['next'].dereference()
>> +
>> +
>> +def list_for_each_entry(head, gdbtype, member):
>> + for node in list_for_each(head):
>> + if node.type != list_head.get_type().pointer():
>> + raise TypeError("Type %s found. "
>> + "Expected struct list_head *." % node.type)
>
> Nit, but FWIW, I've adopted the kernel style of always keeping strings
> on one line so they're easily greppable.
Absolutely! good point. Looks like I was trying to make things fit for
the PEP8 tool. Not sure why I didn't pull just the arg to the next line.
Fixed up locally for the next spin.
Also I think as the rest of the kernel python code is using .format() it
probably makes sense to swap that too to match.
>
> -Jeff
>
Thanks for the eyes.
Regards
--
Kieran