STINNER Victor <victor.stin...@haypocalc.com> added the comment:
> 1. The handling of the error in internal_close() isn't really good. What
> I'm also not sure about is whether it is correct to set the error there,
> the old code sometimes did set an error as reaction to internal_close()
> failing and sometimes not.
internal_close() is used in:
- fileio_close(): raise an IOError(errno) on internal_close() failure
- dircheck(): ignore internal_close() failure, whereas Python 3.x raise
a IOError(errno) on internal_close() failure
- file_init(): catch internal_close() failure but don't set an exception,
whereas Python 3.x set a IOError(errno)
- fileio_dealloc(): PySys_WriteStderr(...) in Python 2.x,
PyErr_WriteUnraisable(self) in Python 3.x
But Python 2.x all errors are ignored because the test (errno < 0) is wrong...
I always prefer Python 3.x behaviour: internal_close() is responsible to raise
an exception.
> 2. There is one place in dircheck() where I think that the error should
> be ignored, but I'm not sure about that.
I rarely a good idea to ignore errors, I prefer to raise a different error if
the file can not be closed.
> 3. This patch fixes another issue, and that is that internal_close()
> would call close() even if 'closefd' was not set. This e.g. happens when
> __init__ is called explicitly. This case is missing a test though.
Python 3.x is also affected by this issue: file_init() and dircheck() ignore
self->closefd.
passwd = open("/etc/passwd")
f = _FileIO(fd, closefd=False)
f.__init__(fd, closefd=False) => close passwd file!
> 4. I wonder if internal_close() should reset only the 'fd' field or if
> it should also reset 'readable', 'writable', 'seekable', too, i.e. apart
> from 'weakreflist' do the same as fileio_new() does.
seekable(), readable() and writable() raise an error if the file is closed. I
think it's fine to leave the internal attributes unchanged.
_______________________________________
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue4841>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
