SF bug 801631: file.truncate fault on windows.

file_truncate():  C doesn't define what fflush(fp) does if fp is open
for update, and the preceding I/O operation on fp was input.  On Windows,
fflush() actually changes the current file position then.  Because
Windows doesn't support ftruncate() directly, this not only caused
Python's file.truncate() to change the file position (contra our docs),
it also caused the file not to change size.

Repaired by getting the initial file position at the start, restoring
it at the end, and tossing all the complicated micro-efficiency checks
trying to avoid "provably unnecessary" seeks.  file.truncate() can't
be a frequent operation, and seeking to the current file position has
got to be cheap anyway.

Bugfix candidate.

Lib/test/test_file.py

@@ -132,3 +132,31 @@ else:
     raise TestFailed, 'file.writelines([]) on a closed file should raise a ValueError'
 
 os.unlink(TESTFN)
+
+def bug801631():
+    # SF bug <http://www.python.org/sf/801631>
+    # "file.truncate fault on windows"
+    f = file(TESTFN, 'wb')
+    f.write('12345678901')   # 11 bytes
+    f.close()
+
+    f = file(TESTFN,'rb+')
+    data = f.read(5)
+    if data != '12345':
+        raise TestFailed("Read on file opened for update failed %r" % data)
+    if f.tell() != 5:
+        raise TestFailed("File pos after read wrong %d" % f.tell())
+
+    f.truncate()
+    if f.tell() != 5:
+        raise TestFailed("File pos after ftruncate wrong %d" % f.tell())
+
+    f.close()
+    size = os.path.getsize(TESTFN)
+    if size != 5:
+        raise TestFailed("File size after ftruncate wrong %d" % size)
+
+try:
+    bug801631()
+finally:
+    os.unlink(TESTFN)

Misc/NEWS

@@ -94,6 +94,10 @@ Tests
 Windows
 -------
 
+- file.truncate() could misbehave if the file was open for update
+  (modes r+, rb+, w+, wb+), and the most recent file operation before
+  the truncate() call was an input operation.  SF bug 801631.
+
 Mac
 ----
 

Objects/fileobject.c

@@ -529,18 +529,33 @@ file_seek(PyFileObject *f, PyObject *args)
 static PyObject *
 file_truncate(PyFileObject *f, PyObject *args)
 {
-	int ret;
 	Py_off_t newsize;
-	PyObject *newsizeobj;
+	PyObject *newsizeobj = NULL;
+	Py_off_t initialpos;
+	int ret;
 
 	if (f->f_fp == NULL)
 		return err_closed();
-	newsizeobj = NULL;
 	if (!PyArg_UnpackTuple(args, "truncate", 0, 1, &newsizeobj))
 		return NULL;
 
+	/* Get current file position.  If the file happens to be open for
+	 * update and the last operation was an input operation, C doesn't
+	 * define what the later fflush() will do, but we promise truncate()
+	 * won't change the current position (and fflush() *does* change it
+	 * then at least on Windows).  The easiest thing is to capture
+	 * current pos now and seek back to it at the end.
+	 */
+	Py_BEGIN_ALLOW_THREADS
+	errno = 0;
+	initialpos = _portable_ftell(f->f_fp);
+	Py_END_ALLOW_THREADS
+	if (initialpos == -1)
+		goto onioerror;
+
 	/* Set newsize to current postion if newsizeobj NULL, else to the
-	   specified value. */
+	 * specified value.
+	 */
 	if (newsizeobj != NULL) {
 #if !defined(HAVE_LARGEFILE_SUPPORT)
 		newsize = PyInt_AsLong(newsizeobj);
@@ -552,17 +567,13 @@ file_truncate(PyFileObject *f, PyObject *args)
 		if (PyErr_Occurred())
 			return NULL;
 	}
-	else {
-		/* Default to current position. */
-		Py_BEGIN_ALLOW_THREADS
-		errno = 0;
-		newsize = _portable_ftell(f->f_fp);
-		Py_END_ALLOW_THREADS
-		if (newsize == -1)
-			goto onioerror;
-	}
+	else /* default to current position */
+		newsize = initialpos;
 
-	/* Flush the file. */
+	/* Flush the stream.  We're mixing stream-level I/O with lower-level
+	 * I/O, and a flush may be necessary to synch both platform views
+	 * of the current file state.
+	 */
 	Py_BEGIN_ALLOW_THREADS
 	errno = 0;
 	ret = fflush(f->f_fp);
@@ -574,66 +585,47 @@ file_truncate(PyFileObject *f, PyObject *args)
 	/* MS _chsize doesn't work if newsize doesn't fit in 32 bits,
 	   so don't even try using it. */
 	{
-		Py_off_t current;	/* current file position */
 		HANDLE hFile;
-		int error;
 
-		/* current <- current file postion. */
-		if (newsizeobj == NULL)
-			current = newsize;
-		else {
+		/* Have to move current pos to desired endpoint on Windows. */
 		Py_BEGIN_ALLOW_THREADS
 		errno = 0;
-			current = _portable_ftell(f->f_fp);
+		ret = _portable_fseek(f->f_fp, newsize, SEEK_SET) != 0;
 		Py_END_ALLOW_THREADS
-			if (current == -1)
+		if (ret)
 			goto onioerror;
-		}
-
-		/* Move to newsize. */
-		if (current != newsize) {
-			Py_BEGIN_ALLOW_THREADS
-			errno = 0;
-			error = _portable_fseek(f->f_fp, newsize, SEEK_SET)
-				!= 0;
-			Py_END_ALLOW_THREADS
-			if (error)
-				goto onioerror;
-		}
 
 		/* Truncate.  Note that this may grow the file! */
 		Py_BEGIN_ALLOW_THREADS
 		errno = 0;
 		hFile = (HANDLE)_get_osfhandle(fileno(f->f_fp));
-		error = hFile == (HANDLE)-1;
-		if (!error) {
-			error = SetEndOfFile(hFile) == 0;
-			if (error)
+		ret = hFile == (HANDLE)-1;
+		if (ret == 0) {
+			ret = SetEndOfFile(hFile) == 0;
+			if (ret)
 				errno = EACCES;
 		}
 		Py_END_ALLOW_THREADS
-		if (error)
-			goto onioerror;
-
-		/* Restore original file position. */
-		if (current != newsize) {
-			Py_BEGIN_ALLOW_THREADS
-			errno = 0;
-			error = _portable_fseek(f->f_fp, current, SEEK_SET)
-				!= 0;
-			Py_END_ALLOW_THREADS
-			if (error)
+		if (ret)
 			goto onioerror;
 	}
-	}
 #else
 	Py_BEGIN_ALLOW_THREADS
 	errno = 0;
 	ret = ftruncate(fileno(f->f_fp), newsize);
 	Py_END_ALLOW_THREADS
-	if (ret != 0) goto onioerror;
+	if (ret != 0)
+		goto onioerror;
 #endif /* !MS_WINDOWS */
 
+	/* Restore original file position. */
+	Py_BEGIN_ALLOW_THREADS
+	errno = 0;
+	ret = _portable_fseek(f->f_fp, initialpos, SEEK_SET) != 0;
+	Py_END_ALLOW_THREADS
+	if (ret)
+		goto onioerror;
+
 	Py_INCREF(Py_None);
 	return Py_None;