Add exception handling in mountinfo enumeration path #1515
Conversation
| # invalid superblocks being return | ||
| # mnt_parent can also smear so we check both | ||
| try: | ||
| parent_id: int = mnt.mnt_parent.mnt_id |
There was a problem hiding this comment.
mnt_parent is a pointer.. this should be:
if not (mnt.mnt_parent and mnt.mnt_parent.is_readable()):
return NoneSame thing for superblock:
def get_mnt_sb(self) -> int:
"""Returns a pointer to the super_block"""
There was a problem hiding this comment.
I would generally err towards testing before accepting (especially if there's gonna be extra code required either way). I've tried in this project to be a little happier about exception handling, although ideally they'd only occur in exceptional circumstances not expected ones. The good part about exception handling is that the try/catch can cover much larger chunks of code regardless of what exactly the issue was, so I'm trying to encourage try/except blocks that cover as much as could go wrong if something were to go wrong (but no more than that) so usually the body of a loop for instance, or something of that nature.
I'm gonna leave you two to figure out what's best ask permission/seek forgiveness, once you come to a consensus let me know, at the moment either one seems ok to me unless someone presents an argument as to why one is better or actively detrimental...
There was a problem hiding this comment.
This looks ok to me, but I'll leave @atcuno and @gcmoreira to come to a compromise they're both happy with and then go with that...
| # invalid superblocks being return | ||
| # mnt_parent can also smear so we check both | ||
| try: | ||
| parent_id: int = mnt.mnt_parent.mnt_id |
There was a problem hiding this comment.
I would generally err towards testing before accepting (especially if there's gonna be extra code required either way). I've tried in this project to be a little happier about exception handling, although ideally they'd only occur in exceptional circumstances not expected ones. The good part about exception handling is that the try/catch can cover much larger chunks of code regardless of what exactly the issue was, so I'm trying to encourage try/except blocks that cover as much as could go wrong if something were to go wrong (but no more than that) so usually the body of a loop for instance, or something of that nature.
I'm gonna leave you two to figure out what's best ask permission/seek forgiveness, once you come to a consensus let me know, at the moment either one seems ok to me unless someone presents an argument as to why one is better or actively detrimental...
The lack of exception handling caused backtraces in many samples in mass testing.