classification
Title: Wrong tell() result for a file opened in append mode
Type: behavior Stage:
Components: Versions: Python 3.0, Python 3.1
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: pitrou, vstinner
Priority: release blocker Keywords: patch

Created on 2009-01-20 00:49 by vstinner, last changed 2009-01-21 01:06 by pitrou. This issue is now closed.

Files
File name Uploaded Description Edit
fileio_append-4.patch vstinner, 2009-01-21 00:27
Messages (9)
msg80230 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2009-01-20 00:49
The following code must display 3 instead of 0:
---
with open("x", "w") as f:
    f.write("xxx")
with open("x", "a") as f:
    print(f.tell())
---

The example works with Python 2.x, because file object is implemented 
using the FILE structure (fopen, ftell, etc.). fopen() "fixes" the 
offset if the file is opened in append mode, whereas open() doesn't do 
this for us :
---
import os
with open("x", "w") as f:
    f.write("xxx")
fd = os.open("x", os.O_RDONLY | os.O_APPEND)
print(os.lseek(fd, 0, 1))
---
display 0 instead of 3 on Python 2.x and 3.x.

It becomes a little bit more weird when you write something :-)
---
with open("x", "w") as f:
    f.write("xxx")
with open("x", "a") as f:
    f.write("y")
    print(f.tell())
---
displays... 4 (the correct position) on Python 2.x and 3.x.

I see (in GNU libc source code) that fopen() call lseek(fd, 0, 
SEEK_END) if the file is opened in append mode.
msg80231 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2009-01-20 01:00
Patch _including a test_:

+	if (append)
+		lseek(self->fd, 0, SEEK_END);
msg80248 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2009-01-20 11:40
Comments on the patch:
- you should check the error return of lseek() (and possibly wrap it in
Py_BEGIN/END_ALLOW_THREADS, see portable_lseek() in the same file)
- there should be a test for each of unbuffered IO (buffering=0),
buffered IO ("rb") and text IO ("r"). For text IO, the test shouldn't
test the actual value returned by tell(), only that it is > 0 (because
tell() in text mode is an opaque value and is not necessarily equal to a
byte position)
msg80294 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2009-01-20 23:30
Patch version 2:
 - raise raise PyErr_SetFromErrno(PyExc_IOError) on lseek() error
 - add tests for unbuffered binary file and (buffered) text file

I use the type "long" to store the lseek() result, because I don't 
know if off_t is available on all OS. Py_off_t may be used, but it's 
defined above (after fileio_init). fileio_seekable() uses the 
type "int" for lseek() result, which looks worse than long :-)
msg80296 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2009-01-20 23:40
> I use the type "long" to store the lseek() result, because I don't 
> know if off_t is available on all OS. Py_off_t may be used, but it's 
> defined above (after fileio_init).

Instead of checking the return type, you can first set errno to 0, and
then check errno after the function returns.

> fileio_seekable() uses the 
> type "int" for lseek() result, which looks worse than long :-)

Nice catch! http://bugs.python.org/issue5016

PS : about the patch, "0 < f.tell()" is really strange coding style...
"f.tell() > 0" looks much more consistent with the rest of the Python
code base
msg80297 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2009-01-20 23:47
Last thing, in your patch there is a forward declaration to
portable_lseek but it doesn't look used anywhere...
msg80298 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2009-01-21 00:07
New try (version 3):
 - reuse Py_off_t in fileio_init() instead of long
 - use Python coding style: f.tell() > 0
 - remove the unused forward declaration of portable_lseek()

This patch also prepares a fix for #5016.
msg80300 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2009-01-21 00:27
Version 4: ok, let's use *portable*_lseek() instead of the "ugly" 
lseek() function (not compatible with large files on Windows).
msg80305 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2009-01-21 01:06
Committed in r68835, r68836, r68837, r68838.
History
Date User Action Args
2009-01-21 01:06:34pitrousetstatus: open -> closed
resolution: accepted -> fixed
messages: + msg80305
2009-01-21 00:38:36pitrousetresolution: accepted
2009-01-21 00:27:43vstinnersetfiles: + fileio_append-4.patch
messages: + msg80300
2009-01-21 00:26:24vstinnersetfiles: - fileio_append-3.patch
2009-01-21 00:07:44vstinnersetfiles: - fileio_append-2.patch
2009-01-21 00:07:42vstinnersetfiles: - fileio_append.patch
2009-01-21 00:07:36vstinnersetfiles: + fileio_append-3.patch
messages: + msg80298
2009-01-20 23:47:33pitrousetmessages: + msg80297
2009-01-20 23:40:33pitrousetmessages: + msg80296
2009-01-20 23:30:40vstinnersetfiles: + fileio_append-2.patch
messages: + msg80294
2009-01-20 11:40:45pitrousetnosy: + pitrou
messages: + msg80248
2009-01-20 01:02:27vstinnersetfiles: + fileio_append.patch
2009-01-20 01:01:40vstinnersetfiles: - fileio_append.patch
2009-01-20 01:00:37vstinnersetfiles: + fileio_append.patch
keywords: + patch
messages: + msg80231
2009-01-20 00:53:56pitrousetpriority: release blocker
type: behavior
2009-01-20 00:49:29vstinnercreate