-
-
Notifications
You must be signed in to change notification settings - Fork 29.2k
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
SystemError if custom opener returns -1 #71253
Comments
Let's say you use a custom opener, and that opener happens to return exactly -1. You end up with a SystemError because NULL got returned without an exception being set: def negative(fname, flags):
return -1
with open('/tmp/foo.txt', 'w', encoding='utf-8', opener=negative) as fp:
print('oops', file=fp) % python3 /tmp/foo.py
Traceback (most recent call last):
File "/tmp/foo.py", line 5, in <module>
with open('/tmp/foo.txt', 'w', encoding='utf-8', opener=negative) as fp:
SystemError: <class '_io.FileIO'> returned NULL without setting an error Anything else and you get a relatively decent exception. E.g. return -2 and you get an OSError. Raise an exception and you get that exception. The problem is pretty clear to see; when an opener is set, after coercing the fd to an integer, the check is made for that integer being -1, and then it jumps right to the exit. Let's say you return some non-integer, like 'foo'. Then the _PyLong_AsInt() will fail and a proper exception will be set. So I think the "if (self->fd == -1)" clause just needs to check for an exception set first and set one if there isn't one before it does the "goto error". I guess you'd want to see the same exception as if it returned say, -2: Traceback (most recent call last):
File "/tmp/foo.py", line 5, in <module>
with open('/tmp/foo.txt', 'w', encoding='utf-8', opener=negative) as fp:
OSError: [Errno 0] Error: '/tmp/foo.txt' |
Also, |
On May 20, 2016, at 12:12 AM, ppperry wrote:
No, definitely not. ;) |
Here's a proposed fix. |
Added comments on Rietveld. |
In general LGTM, but I left style comment on Rietveld. And you can use assertRaisesRegex in tests if you prefers. |
BTW, I may wait to commit this until after we've moved to GitHub. :) |
New changeset 4af64ab34eef by Barry Warsaw in branch '3.5': New changeset 84c91d7d4667 by Barry Warsaw in branch 'default': |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: