Issue5008
Created on 2009-01-20 00:49 by vstinner, last changed 2022-04-11 14:56 by admin. 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) * ![]() |
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) * ![]() |
Date: 2009-01-20 01:00 | |
Patch _including a test_: + if (append) + lseek(self->fd, 0, SEEK_END); |
|||
| msg80248 - (view) | Author: Antoine Pitrou (pitrou) * ![]() |
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) * ![]() |
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) * ![]() |
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) * ![]() |
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) * ![]() |
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) * ![]() |
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) * ![]() |
Date: 2009-01-21 01:06 | |
Committed in r68835, r68836, r68837, r68838. |
|||
| History | |||
|---|---|---|---|
| Date | User | Action | Args |
| 2022-04-11 14:56:44 | admin | set | nosy:
+ benjamin.peterson github: 49258 |
| 2009-01-21 01:06:34 | pitrou | set | status: open -> closed resolution: accepted -> fixed messages: + msg80305 |
| 2009-01-21 00:38:36 | pitrou | set | resolution: accepted |
| 2009-01-21 00:27:43 | vstinner | set | files:
+ fileio_append-4.patch messages: + msg80300 |
| 2009-01-21 00:26:24 | vstinner | set | files: - fileio_append-3.patch |
| 2009-01-21 00:07:44 | vstinner | set | files: - fileio_append-2.patch |
| 2009-01-21 00:07:42 | vstinner | set | files: - fileio_append.patch |
| 2009-01-21 00:07:36 | vstinner | set | files:
+ fileio_append-3.patch messages: + msg80298 |
| 2009-01-20 23:47:33 | pitrou | set | messages: + msg80297 |
| 2009-01-20 23:40:33 | pitrou | set | messages: + msg80296 |
| 2009-01-20 23:30:40 | vstinner | set | files:
+ fileio_append-2.patch messages: + msg80294 |
| 2009-01-20 11:40:45 | pitrou | set | nosy:
+ pitrou messages: + msg80248 |
| 2009-01-20 01:02:27 | vstinner | set | files: + fileio_append.patch |
| 2009-01-20 01:01:40 | vstinner | set | files: - fileio_append.patch |
| 2009-01-20 01:00:37 | vstinner | set | files:
+ fileio_append.patch keywords: + patch messages: + msg80231 |
| 2009-01-20 00:53:56 | pitrou | set | priority: release blocker type: behavior |
| 2009-01-20 00:49:29 | vstinner | create | |
