Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Devel/rbtree #39

Merged
merged 3 commits into from
Nov 17, 2020
Merged

Devel/rbtree #39

merged 3 commits into from
Nov 17, 2020

Conversation

Werkov
Copy link
Contributor

@Werkov Werkov commented Jun 27, 2019

No description provided.

Copy link
Member

@jeffmahoney jeffmahoney left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are a few nits, comments inline, but also (as you mentioned on IRC) since this is intended to be part of the API, we need documentation for it. We also need test cases. If you can come up with simple cases like those in tests/test_list.py, that would be helpful. Test cases that work with common in-kernel rbtrees would also be helpful to ensure that it still works correctly when the kernel is updated.

Finally, outside of the .gitignore changes, this should all be a single commit.

Thanks,

-Jeff

if root.type == types.rb_root_type.pointer():
root = root.dereference()
elif root.type != types.rb_root_type:
raise ArgumentTypeError('root', root, types.rb_root_type)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be UnexpectedGDBTypeError now.


def rbtree_postorder_for_each(root: gdb.Value) -> Iterable[gdb.Value]:
if not isinstance(root, gdb.Value):
raise InvalidArgumentError("root must be gdb.Value representing 'struct rb_root' or 'struct rb_root *' not {}"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be ArgumentTypeError now

@Werkov
Copy link
Contributor Author

Werkov commented Jun 28, 2019

No ready yet (only backing up the branch), will rebase later.

@Werkov
Copy link
Contributor Author

Werkov commented Jul 2, 2019

Force pushed a new version:

  • rebased onto current master,
  • added unit and dump tests,
  • squashed altogether.

Also, I noticed that similar API for linked list traversing says in the docs:

def list_for_each
[...]
gdb.Value: The next node in the list. The value is
of type struct list_head.

However, the returned gdb.Value is of type struct list_head *, i.e. the docs isn't consistent with the implementation. The rbtree implementation honors the docs. Let me know which is the preferred way for these iterators (I expect the non-pointer variant).

Werkov and others added 3 commits July 8, 2020 12:43
Signed-off-by: Michal Koutný <mkoutny@suse.com>
Add functions for traversing generic RB tree structures and accompanying
tests (both for correctness and applicability to real kernel dumps).

Signed-off-by: Jeff Mahoney <jeffm@suse.com>
Signed-off-by: Michal Koutný <mkoutny@suse.com>
We honor the principle of accepting both pointer to or object itself on
input and giving out the object on output. The list iterator
implementation returns the pointer (contradicting docs). Switch to the
object type for consistency with other APIs (e.g. list_for_each_entry).

Signed-off-by: Michal Koutný <mkoutny@suse.com>
@Werkov
Copy link
Contributor Author

Werkov commented Jul 8, 2020

I've rebased the branch on the current master (resolved trouble with gdb update from 8.3 to 9.1) and added one more commit to rectify the type situation as laid out in my previous comment.
AFAICT, all the remarks should be resolved now.

@Werkov Werkov mentioned this pull request Jul 8, 2020
@jeffmahoney jeffmahoney merged commit 47567cc into crash-python:master Nov 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants