python: fix iterators failing on empty lists#1067
Conversation
The python bindings implement hosts(), subsystems(), controllers(), and namespaces() as iterators. However, these iterators all fail on empty lists. This hopefully fixes this issue. Signed-off-by: Martin Belanger <[email protected]>
martin-belanger
force-pushed
the
python-iterators
branch
from
b8862e3 to
890d7a5
Compare
|
That looks indeed far cleaner. Have you checked whether you can remove the iter elements? They should not be needed now. |
I will try removing the |
|
No, they are not. I've removed them on my local branch and everything worked. Would've pushed them but that would have created a new PR I guess, so I didn't. |
This is just a bit of clean up. After reworking iterators by using actual Python code instead of C++, we don't need to keep these "iter" elements: host_iter, subsystem_iter, ctrl_iter, ns_iter. Signed-off-by: Martin Belanger <[email protected]>
martin-belanger
changed the title
|
@hreinecke @igaw - Done with the changes. Removed the WIP label on this PR. |
hreinecke
left a comment
hreinecke
left a comment
There was a problem hiding this comment.
That looks good to me.
| struct nvme_ns * nvme_subsystem_first_ns(struct nvme_subsystem * s); | ||
| struct nvme_ns * nvme_subsystem_next_ns(struct nvme_subsystem * s, struct nvme_ns * n); | ||
| struct nvme_ns * nvme_ctrl_first_ns(struct nvme_ctrl * c); | ||
| struct nvme_ns * nvme_ctrl_next_ns(struct nvme_ctrl * c, struct nvme_ns * n); |
There was a problem hiding this comment.
Why are these deceleration necessary? I see we include tree.h right above.
There was a problem hiding this comment.
I see that python-sigsegv-during-gc fails without these deceleration. I think I don't understand how the SWIG compiler works :)
|
SWIG is a weird beast... There are two types of includes.
The first one is to be added to the generated C/C++ code. This is so that the generated C/C++ code will find the definitions and prototypes that it needs to build the C/C++ code. The second one is to tell SWIG that we want to generate Python wrappers for everything found in the included file. Since we don't want to generate Python wrappers for everything in tree.h, we don't use this type of %include. Instead, we list the exact functions that we want to wrap. This is the part that I find a bit annoying with SWIG. It would be simpler if we just wrapped everything, but then the generated code would contain a lot of things we don't really need in Python. We could also use a SWIG flag ( |
|
Thanks for explaining. Yeah, SWIG seems tricky to use. |
|
Thanks! And I do like to get rid of these C++ iters. First I thought it's odd to add Python code but given we can get rid of this C++, I am for it! |
3 participants
This is my attempt to fix the issue reported by @hreinecke: #1064
With this PR,
subsystems(),controllers(), etc, are now implemented as native Python iterators (usingyield).After confirming with Hannes I also removed all the
iterelements (host_iter,subsystem_iter, etc) that were no longer needed. I kept the removal of theiterelements as a separate commit just in case we need to revert the removal.