Prevent infinite looping and out of memory errors #1482#1490
Conversation
|
This is ready @ikelos |
ikelos
left a comment
ikelos
left a comment
There was a problem hiding this comment.
Happy with the error checking, just wondering how best to indicate the error to the user? Possibly we should have a vollog warning that prints as much of the key name as could be recovered? I'm not sure...
Also, if you want github to auto tag the issues these are supposed to fix, you can't just mention it in the title, you need to put "Fixes #blah" or "Closes #blah" in the bug body, please...
| kcb = self.KeyControlBlock | ||
| while kcb.ParentKcb: | ||
| if kcb.ParentKcb.vol.offset in seen: | ||
| return "" |
There was a problem hiding this comment.
Shouldn't these be None or something else? I don't want the BaseAbsentValues sneaking in here, but it feels like we should be alerting people that things didn't work? The downside with that is then you have to do error checking whenever you try to pull the full key name? Just wondering of the consequences of it returning a value but blank string in case of an error?
There was a problem hiding this comment.
That function is called from handles on this line:
Which, in the else case of that same block, the empty string is used to indicate an error:
If it makes more sense, I can change both places (the registry extension + line 318 in handles) to send back None instead of "", then have the yield() handler do the "variable or renderers....." setup. Is that preferred? That avoids the extension API having to send back a renderer instance.
There was a problem hiding this comment.
Also, thanks for this:
"Also, if you want github to auto tag the issues these are supposed to fix, you can't just mention it in the title, you need to put "Fixes #blah" or "Closes #blah" in the bug body, please..."
I am used to GitLab where tagging the number accomplishes it.
There was a problem hiding this comment.
Yeah, I think it would be better to return None as a way of indicating an error, and then let the caller figure out what to do with it. Also, no problem. 5;)
atcuno
force-pushed
the
issue_1482_infinite_loop_oom
branch
from
3aa4af8 to
e9088be
Compare
|
@ikelos this is ready again |
2 participants
No description provided.