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

Linux: Extensions - removes abc.Iterable definition #1320

Conversation

dgmcdona
Copy link
Contributor

hlist_head is missing an implementation for collections.abc.Iterable, resulting in a crash at runtime. This fixes the issue by providing the required __iter__ implementation for hlist_head.

`hlist_head` is missing an implementation for
`collections.abc.Iterable`, resulting in a crash at runtime. This fixes
the issue by providing the required `__iter__` implementation for
`hlist_head`.
@gcmoreira
Copy link
Contributor

Hey @dgmcdona it was just removed :) ... good catch anyway but the proposed fix is not correct. It's actually the other way around, we have to remove collections.abc.Iterable from the class declaration, I forgot to do this, my bad.
self.vol.member_name cannot be used as the member name to iterate this type and that's why a __iter__ doesn't make sense here.

@gcmoreira
Copy link
Contributor

gcmoreira commented Oct 24, 2024

By the way, __iter__ doesn’t make sense for list_head either, so it also needs to be removed from that object as well. I'll handle that in a separate PR soon, but it will need more extensive testing since many plugins rely on that object.

@dgmcdona
Copy link
Contributor Author

Sounds good, thanks @gcmoreira ! I'll close this one, then.

@dgmcdona dgmcdona closed this Oct 24, 2024
@gcmoreira
Copy link
Contributor

Oh we can fix this and take advantage of your finding here if you want. You should remove the hlist_head __iter__ method and collections.abc.Iterable from the class declaration and it will work again. Sorry for the misunderstanding, I didn't mean you should close it.

@dgmcdona dgmcdona reopened this Oct 24, 2024
Per @gcmoreira this is the correct fix vs the one previously proposed,
since self.vol.member_name cannot be used as the member name to
iterate this type.
@dgmcdona dgmcdona changed the title Linux: Extensions - Adds missing abc.Iterable implementation Linux: Extensions - removes abc.Iterable implementation Oct 24, 2024
@dgmcdona dgmcdona changed the title Linux: Extensions - removes abc.Iterable implementation Linux: Extensions - removes abc.Iterable definition Oct 24, 2024
@gcmoreira
Copy link
Contributor

Great! thanks @dgmcdona, it looks good.

@ikelos this is ready for your review. @dgmcdona caught this bug right away 🥇 . I messed it up in the last commit.

Copy link
Member

@ikelos ikelos left a comment

Choose a reason for hiding this comment

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

Happy if you guys are...

@ikelos ikelos merged commit a196140 into volatilityfoundation:develop Oct 24, 2024
12 checks passed
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.

3 participants