[#521782] unreliable file.read() error handling
* Objects/fileobject.c
(file_read): Clear errors before leaving the loop in all situations,
and also check if some data was read before exiting the loop with an
EWOULDBLOCK exception.
* Doc/lib/libstdtypes.tex
* Objects/fileobject.c
Document that sometimes a read() operation can return less data than
what the user asked, if running in non-blocking mode.
* Misc/NEWS
Document the fix.
For a file f, iter(f) now returns f (unless f is closed), and f.next()
is similar to f.readline() when EOF is not reached; however, f.next()
uses a readahead buffer that messes up the file position, so mixing
f.next() and f.readline() (or other methods) doesn't work right.
Calling f.seek() drops the readahead buffer, but other operations
don't.
The real purpose of this change is to reduce the confusion between
objects and their iterators. By making a file its own iterator, it's
made clearer that using the iterator modifies the file object's state
(in particular the current position).
A nice side effect is that this speeds up "for line in f:" by not
having to use the xreadlines module. The f.xreadlines() method is
still supported for backwards compatibility, though it is the same as
iter(f) now.
(I made some cosmetic changes to Oren's code, and added a test for
"file closed" to file_iternext() and file_iter().)
MSDN sample programs use it, apparently in error. The correct name
is WIN32_LEAN_AND_MEAN. After switching to the correct name, in two
cases more was needed because the code actually relied on things that
disappear when WIN32_LEAN_AND_MEAN is defined.
don't understand how this function works, also beefed up the docs. The
most common usage error is of this form (often spread out across gotos):
if (_PyString_Resize(&s, n) < 0) {
Py_DECREF(s);
s = NULL;
goto outtahere;
}
The error is that if _PyString_Resize runs out of memory, it automatically
decrefs the input string object s (which also deallocates it, since its
refcount must be 1 upon entry), and sets s to NULL. So if the "if"
branch ever triggers, it's an error to call Py_DECREF(s): s is already
NULL! A correct way to write the above is the simpler (and intended)
if (_PyString_Resize(&s, n) < 0)
goto outtahere;
Bugfix candidate.
+ Continued looping until n bytes in the buffer have been filled, not
just when n bytes have been read from the file. This repairs the
bug that f.readlines() only sucked up the first 8192 bytes of the file
on Windows when universal newlines was enabled and f was opened in
U mode (see Python-Dev -- this was the ultimate cause of the
test_inspect.py failure).
+ Changed prototye to take a char* buffer (void* doesn't make much sense).
+ Squashed size_t vs int mismatches (in particular, besides the unsigned
vs signed distinction, size_t may be larger than int).
+ Gets out under all error conditions now (it's possible for fread() to
suffer an error even if it returns a number larger than 0 -- any
"short read" is an error or EOF condition).
+ Rearranged and simplified declarations.
Highlights: import and friends will understand any of \r, \n and \r\n
as end of line. Python file input will do the same if you use mode 'U'.
Everything can be disabled by configuring with --without-universal-newlines.
See PEP278 for details.
open_the_file: Some (not all) flavors of Windows set errno to EINVAL
when passed a syntactically invalid filename. Python turned that into an
incomprehensible complaint about the mode string. Fixed by special-casing
MSVC.
PEP 285. Everything described in the PEP is here, and there is even
some documentation. I had to fix 12 unit tests; all but one of these
were printing Boolean outcomes that changed from 0/1 to False/True.
(The exception is test_unicode.py, which did a type(x) == type(y)
style comparison. I could've fixed that with a single line using
issubtype(x, type(y)), but instead chose to be explicit about those
places where a bool is expected.
Still to do: perhaps more documentation; change standard library
modules to return False/True from predicates.
of get_line. This makes test_bufio finish in 1.7 seconds instead of 57
seconds on my machine (with Py_DEBUG defined).
Also, rename the local variables n1 and n2 to used_v_size and
total_v_size.
When WITH_PYMALLOC is defined, define PYMALLOC_DEBUG to enable the debug
allocator. This can be done independent of build type (release or debug).
A debug build automatically defines PYMALLOC_DEBUG when pymalloc is
enabled. It's a detected error to define PYMALLOC_DEBUG when pymalloc
isn't enabled.
Two debugging entry points defined only under PYMALLOC_DEBUG:
+ _PyMalloc_DebugCheckAddress(const void *p) can be used (e.g., from gdb)
to sanity-check a memory block obtained from pymalloc. It sprays
info to stderr (see next) and dies via Py_FatalError if the block is
detectably damaged.
+ _PyMalloc_DebugDumpAddress(const void *p) can be used to spray info
about a debug memory block to stderr.
A tiny start at implementing "API family" checks isn't good for
anything yet.
_PyMalloc_DebugRealloc() has been optimized to do little when the new
size is <= old size. However, if the new size is larger, it really
can't call the underlying realloc() routine without either violating its
contract, or knowing something non-trivial about how the underlying
realloc() works. A memcpy is always done in this case.
This was a disaster for (and only) one of the std tests: test_bufio
creates single text file lines up to a million characters long. On
Windows, fileobject.c's get_line() uses the horridly funky
getline_via_fgets(), which keeps growing and growing a string object
hoping to find a newline. It grew the string object 1000 bytes each
time, so for a million-character string it took approximately forever
(I gave up after a few minutes).
So, also:
fileobject.c, getline_via_fgets(): When a single line is outrageously
long, grow the string object at a mildly exponential rate, instead of
just 1000 bytes at a time.
That's enough so that a debug-build test_bufio finishes in about 5 seconds
on my Win98SE box. I'm curious to try this on Win2K, because it has very
different memory behavior than Win9X, and test_bufio always took a factor
of 10 longer to complete on Win2K. It *could* be that the endless
reallocs were simply killing it on Win2K even in the release build.
and (b) stop trying to prevent file growth.
Beef up the file.truncate() docs.
Change test_largefile.py to stop assuming that f.truncate() moves the
file pointer to the truncation point, and to verify instead that it leaves
the file position alone. Remove the test for what happens when a
specified size exceeds the original file size (it's ill-defined, according
to the Single Unix Spec).
dropping MS's inadequate _chsize() function. This was inspired by
SF patch 498109 ("fileobject truncate support for win32"), which I
rejected.
libstdtypes.tex: Someone who knows should update the availability
blurb. For example, if it's available on Linux, it would be good to
say so.
test_largefile: Uncommented the file.truncate() tests, and reworked to
do more. The old comment about "permission errors" in the truncation
tests under Windows was almost certainly due to that the file wasn't open
for *write* access at this point, so of course MS wouldn't let you
truncate it. I'd be appalled if a Unixish system did.
CAUTION: Someone should run this test on Linux (etc) too. The
truncation part was commented out before. Note that test_largefile isn't
run by default.
Objects/
fileobject.c
stringobject.c
unicodeobject.c
This commit doesn't include the cleanup patches for stringobject.c and
unicodeobject.c which are shown separately in the patch manager. Those
patches will be regenerated and applied in a subsequent commit, so as
to preserve a fallback position (this commit to those files).
const char* instead of char*. The change is conceptually correct, and
indirectly fixes a compiler wng introduced when somebody else innocently
passed a const char* to this function.
object, so the "Metroworks only" section should not decref it in case
of error (the caller is responsible for decref'ing in case of error --
and does).
The C-code in fileobject.readinto(buffer) which parses
the arguments assumes that size_t is interchangeable
with int:
size_t ntodo, ndone, nnow;
if (f->f_fp == NULL)
return err_closed();
if (!PyArg_Parse(args, "w#", &ptr, &ntodo))
return NULL;
This causes a problem on Alpha / Tru64 / OSF1 v5.1
where size_t is a long and sizeof(long) != sizeof(int).
The patch I'm proposing declares ntodo as an int. An
alternative might be to redefine w# to expect size_t.
[We can't change w# because there are probably third party modules
relying on it. GvR]
The problem is that if fread() returns a short count, we attempt
another fread() the next time through the loop, and apparently glibc
clears or ignores the eof condition so the second fread() requires
another ^D to make it see the eof condition.
According to the man page (and the C std, I hope) fread() can only
return a short count on error or eof. I'm using that in the band-aid
solution to avoid calling fread() a second time after a short read.
Note that xreadlines() still has this problem: it calls
readlines(sizehint) until it gets a zero-length return. Since
xreadlines() is mostly used for reading real files, I won't worry
about this until we get a bug report.
lseek(fp, 0L, SEEK_CUR) can make a filedescriptor unusable.
This workaround is expected to last only a few weeks (until GUSI
is fixed), but without it test_email fails.
many types were subclassable but had a xxx_dealloc function that
called PyObject_DEL(self) directly instead of deferring to
self->ob_type->tp_free(self). It is permissible to set tp_free in the
type object directly to _PyObject_Del, for non-GC types, or to
_PyObject_GC_Del, for GC types. Still, PyObject_DEL was a tad faster,
so I'm fearing that our pystone rating is going down again. I'm not
sure if doing something like
void xxx_dealloc(PyObject *self)
{
if (PyXxxCheckExact(self))
PyObject_DEL(self);
else
self->ob_type->tp_free(self);
}
is any faster than always calling the else branch, so I haven't
attempted that -- however those types whose own dealloc is fancier
(int, float, unicode) do use this pattern.
no backwards compatibility to worry about, so I just pushed the
'closure' struct member to the back -- it's never used in the current
code base (I may eliminate it, but that's more work because the getter
and setter signatures would have to change.)
As examples, I added actual docstrings to the getset attributes of a
few types: file.closed, xxsubtype.spamdict.state.
compatibility, this required all places where an array of "struct
memberlist" structures was declared that is referenced from a type's
tp_members slot to change the type of the structure to PyMemberDef;
"struct memberlist" is now only used by old code that still calls
PyMember_Get/Set. The code in PyObject_GenericGetAttr/SetAttr now
calls the new APIs PyMember_GetOne/SetOne, which take a PyMemberDef
argument.
As examples, I added actual docstrings to the attributes of a few
types: file, complex, instance method, super, and xxsubtype.spamlist.
Also converted the symtable to new style getattr.
A surprising number of changes to split tp_new into tp_new and tp_init.
Turned out the older PyFile_FromFile() didn't initialize the memory it
allocated in all (error) cases, which caused new sanity asserts
elsewhere to fail left & right (and could have, e.g., caused file_dealloc
to try decrefing random addresses).
just by doing type(f) where f is any file object. This left a hole in
restricted execution mode that rexec.py can't plug by itself (although it
can plug part of it; the rest is plugged in fileobject.c now).
Subtlety on Windows: if we change test_largefile.py to use a file
> 4GB, it still fails. A debug session suggests this is because
fseek(fp, 0, 2) refuses to seek to the end of the file when the file
is > 4GB, because it uses the SetFilePointer() in 32-bit mode.
But it only fails when we seek relative to the end of the file,
because in the other seek modes only calls to fgetpos() and fsetpos()
are made, which use Get/SetFilePointer() in 64-bit mode. Solution:
#ifdef MS_WInDOWS, replace the call to fseek(fp, ...) with a call to
_lseeki64(fileno(fp), ...). Make sure to call fflush(fp) first.
(XXX Could also replace the entire branch with a call to _lseeki64().
Would that be more efficient? Certainly less generated code.)
(XXX This needs more testing. I can't actually test that it works for
files >4GB on my Win98 machine, because the filesystem here won't let
me create files >=4GB at all. Tim should test this on his Win2K
machine.)
Curious: the MS docs say stati64 etc are supported even on Win95, but
Win95 doesn't support a filesystem that allows partitions > 2 Gb.
test_largefile: This was opening its test file in text mode. I have no
idea how that worked under Win64, but it sure needs binary mode on Win98.
BTW, on Win98 test_largefile runs quickly (under a second).
I believe this works on Linux (tested both on a system with large file
support and one without it), and it may work on Solaris 2.7.
The changes are twofold:
(1) The configure script now boldly tries to set the two symbols that
are recommended (for Solaris and Linux), and then tries a test
script that does some simple seeking without writing.
(2) The _portable_{fseek,ftell} functions are a little more systematic
in how they try the different large file support options: first
try fseeko/ftello, but only if off_t is large; then try
fseek64/ftell64; then try hacking with fgetpos/fsetpos.
I'm keeping my fingers crossed. The meaning of the
HAVE_LARGEFILE_SUPPORT macro is not at all clear.
I'll see if I can get it to work on Windows as well.
Previously, f.read() and f.readlines() checked for
errors on their file object and possibly raised an
IOError, but f.readline() didn't. This patch makes
f.readline() behave like the others.
Note that I've added a call to clearerr() since the other calls to
ferror() include that too.
I have no way to test this code. :-)
This should be faster.
This means:
(1) "for line in file:" won't work if the xreadlines module can't be
imported.
(2) The body of "for line in file:" shouldn't use the file directly;
the effects (e.g. of file.readline(), file.seek() or even
file.tell()) would be undefined because of the buffering that goes
on in the xreadlines module.
sees it (test_iter.py is unchanged).
- Added a tp_iternext slot, which calls the iterator's next() method;
this is much faster for built-in iterators over built-in types
such as lists and dicts, speeding up pybench's ForLoop with about
25% compared to Python 2.1. (Now there's a good argument for
iterators. ;-)
- Renamed the built-in sequence iterator SeqIter, affecting the C API
functions for it. (This frees up the PyIter prefix for generic
iterator operations.)
- Added PyIter_Check(obj), which checks that obj's type has a
tp_iternext slot and that the proper feature flag is set.
- Added PyIter_Next(obj) which calls the tp_iternext slot. It has a
somewhat complex return condition due to the need for speed: when it
returns NULL, it may not have set an exception condition, meaning
the iterator is exhausted; when the exception StopIteration is set
(or a derived exception class), it means the same thing; any other
exception means some other error occurred.
- In _portable_ftell(), try fgetpos() before ftello() and ftell64().
I ran into a situation on a 64-bit capable Linux where the C
library's ftello() and ftell64() returned negative numbers despite
fpos_t and off_t both being 64-bit types; fgetpos() did the right
thing.
- Define a new typedef, Py_off_t, which is either fpos_t or off_t,
depending on which one is 64 bits. This removes the need for a lot
of #ifdefs later on. (XXX Should this be moved to pyport.h? That
file currently seems oblivious to large fille support, so for now
I'll leave it here where it's needed.)
simpler if we use fgetpos and fsetpos, rather than trying to mess with
platform-specific TELL64 alternatives.
Of course, this hasn't been tested on a 64-bit platform, so I may have
to withdraw this -- but I'm hopeful, and Trent Mick supports this
patch!
faster than the other. Should be faster for Mark Favas's 254-character
mail log lines, and *is* 3-4% quicker for my test case with much shorter
lines (but they're typical of *my* text files, and I'm tired of optimizing
for everyone else at my expense <wink> -- in fact, the only one who loses
here is Guido ...).
Tim discovered another "bug" in my get_line() code: while the comments
said that n<0 was invalid, it was in fact still called with n<0 (when
PyFile_GetLine() was called with n<0). In that case fortunately
executed the same code as for n==0.
Changed the comment to admit this fact, and changed Tim's MS speed
hack code to use 'n <= 0' as the criteria for the speed hack.
code duplication is to let us get away without a realloc whenever possible;
boosted the init buf size (the cutoff at which we *can* get away without
a realloc) from 100 to 200 so that more files can enjoy this boost; and
allowed other threads to run in all cases. The last two cost something,
but not significantly: in my fat test case, less than a 1% slowdown total.
Since my test case has a great many short lines, that's probably the worst
slowdown, too. While the logic barely changed, there were lots of edits.
This also gets rid of the reference to fp->_cnt, so the last platform
assumption being made here is that fgets doesn't overwrite bytes
capriciously (== beyond the terminating null byte it must write).
variant that never needs to "search from the right".
Also fixed unlikely memory leak in get_line, if string size overflows INTMAX.
Also new std test test_bufio to make sure .readline() works.
realized that this behavior is already present in PyFile_GetLine(),
which is the only place that needs it. A little refactoring of that
function made get_line_raw() redundant.
- The raw_input() functionality is moved to a separate function.
- Drop GNU getline() in favor of getc_unlocked(), which exists on more
platforms (and is even a tad faster on my system).
Add definitions of INT_MAX and LONG_MAX to pyport.h.
Remove includes of limits.h and conditional definitions of INT_MAX
and LONG_MAX elsewhere.
This closes SourceForge patch #101659 and bug #115323.
file.writelines() now tries to emulate the behaviour of file.write()
as closely as possible. Due to the problems with releasing the
interpreter lock the solution isn't exactly optimal, but still better
than not supporting the file.write() semantics at all.
The common technique for printing out a pointer has been to cast to a long
and use the "%lx" printf modifier. This is incorrect on Win64 where casting
to a long truncates the pointer. The "%p" formatter should be used instead.
The problem as stated by Tim:
> Unfortunately, the C committee refused to define what %p conversion "looks
> like" -- they explicitly allowed it to be implementation-defined. Older
> versions of Microsoft C even stuck a colon in the middle of the address (in
> the days of segment+offset addressing)!
The result is that the hex value of a pointer will maybe/maybe not have a 0x
prepended to it.
Notes on the patch:
There are two main classes of changes:
- in the various repr() functions that print out pointers
- debugging printf's in the various thread_*.h files (these are why the
patch is large)
Closes SourceForge patch #100505.
For more comments, read the patches@python.org archives.
For documentation read the comments in mymalloc.h and objimpl.h.
(This is not exactly what Vladimir posted to the patches list; I've
made a few changes, and Vladimir sent me a fix in private email for a
problem that only occurs in debug mode. I'm also holding back on his
change to main.c, which seems unnecessary to me.)
This (1) avoids thread unsafety whereby another thread could zap the
list while we were using it, and (2) now supports writing arbitrary
sequences of strings.
buffer increment, and sometimes the new buffer size. Make it do what
its name says, and fix the one place where this matters to the caller.
Also add a comment explaining why we call lseek() and then ftell().
sys.stdin.readline(), you get a fatal error (no current thread). This
is because there was a call to PyErr_CheckSignals() while there was no
current thread. I wonder how many more of these we find... I bnetter
go hunting for PyErr_CheckSignals() now...
this many bytes have been read, readlines stops. Because of
buffering, the amount of bytes read is usually at least 8K more than
the hint.
Also changed read() and readline() to use PyArg_ParseTuple().
(Note that the *previous* checkin also fixed error handling and
narrowed the range of thread unblocking for all methods using
fread().)
see if we can guess the #bytes until the end of the file. If we
can't, increment the buffer size increments up to 0.5Meg to avoid
realloc'ing too much.
* object.[ch], bltinmodule.c, fileobject.c: changed str() to call
strobject() which calls an object's __str__ method if it has one.
strobject() is also called by writeobject() when PRINT_RAW is passed.
* ceval.c: rationalize code for PRINT_ITEM (no change in function!)
* funcobject.c, codeobject.c: added compare and hash functionality.
Functions with identical code objects and the same global dictionary are
equal. Code objects are equal when their code, constants list and names
list are identical (i.e. the filename and code name don't count).
(hash doesn't work yet since the constants are in a list and lists can't
be hashed -- suppose this should really be done with a tuple now we have
resizetuple!)
* Grammar: add exec statement; allow testlist in expr statement.
* ceval.c, compile.c, opcode.h: support exec statement;
avoid optimizing locals when it is used
* fileobject.{c,h}: add getfilename() internal function.
* many files: made some functions static; removed "extern int errno;".
* frozenmain.c: fixed bugs introduced on 24 June...
* flmodule.c: remove 1.5 bw compat hacks, add new functions in 2.2a
(and some old functions that were omitted).
* timemodule.c: added MSDOS floatsleep version .
* pgenmain.c: changed exit() to goaway() and added defn of goaway().
* intrcheck.c: add hack (to UNIX only) so interrupting 3 times
will exit from a hanging program. The second interrupt prints
a message explaining this to the user.
Added $(SYSDEF) to its build rule in Makefile.
* cgensupport.[ch], modsupport.[ch]: removed some old stuff. Also
changed files that still used it... And made several things static
that weren't but should have been... And other minor cleanups...
* listobject.[ch]: add external interfaces {set,get}listslice
* socketmodule.c: fix bugs in new send() argument parsing.
* sunaudiodevmodule.c: added flush() and close().
* various modules: added 1993 to copyright.
* thread.c: added copyright notice.
* ceval.c: minor change to error message for "+"
* stdwinmodule.c: check for error from wfetchcolor
* config.c: MS-DOS fixes (define PYTHONPATH, use DELIM, use osdefs.h)
* Add declaration of inittab to import.h
* sysmodule.c: added sys.builtin_module_names
* xxmodule.c, xxobject.c: fix minor errors
stdwinmodule.c: wsetfont can now return an error
Makefile: add CL_USE and CL_LIB*S; config.c: move CL part around
New things in imgfile; also in Makefile.
longobject.c: fix comparison of negative long ints... [REAL BUG!]
marshal.c: add dumps() and loads() to read/write strings
timemodule.c: make sure there's always a floatsleep()
posixmodule.c: rationalize struct returned by times()
Makefile: add test target, disable imgfile by default
thread.c: Improved coexistance with dl module (sjoerd)
stdwinmodule.c: Change include stdwin.h if macintosh
rotormodule.c: added missing last argument to RTR_?_region calls
confic.c: merged with configmac.c, added 1993 to copyright message
fileobject.c: int compared to NULL in writestring(); change fopenRF ifdef
timemodule.c: simplify times() using mkvalue; include myselect.h
earlier (for sequent).
posixmodule: for sequent, include unistd.h instead of explicit
extern definitions and don't define rename()
Makefile: change misleading/wrong MD5 comments
sys.stderr or sys.stdin, and to work with any object as long as it has
a write() (respectively readline()) methods. Some functions that took
a FILE* argument now take an object* argument.
Gustavo Niemeyer
Martin v. Löwis
Mark Hammond
Jeremy Hylton
Neal Norwitz
Guido van Rossum
Tim Peters
Barry Warsaw
Jack Jansen
Neil Schemenauer
Andrew MacIntyre
Michael W. Hudson
Fred Drake
Andrew M. Kuchling
Marc-André Lemburg
Trent Mick