Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wrong tell() result for a file opened in append mode #49258

Closed
vstinner opened this issue Jan 20, 2009 · 9 comments
Closed

Wrong tell() result for a file opened in append mode #49258

vstinner opened this issue Jan 20, 2009 · 9 comments
Labels
release-blocker type-bug An unexpected behavior, bug, or error

Comments

@vstinner
Copy link
Member

BPO 5008
Nosy @pitrou, @vstinner
Files
  • fileio_append-4.patch
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = <Date 2009-01-21.01:06:34.907>
    created_at = <Date 2009-01-20.00:49:29.627>
    labels = ['type-bug', 'release-blocker']
    title = 'Wrong tell() result for a file opened in append mode'
    updated_at = <Date 2009-01-21.01:06:34.890>
    user = 'https://github.com/vstinner'

    bugs.python.org fields:

    activity = <Date 2009-01-21.01:06:34.890>
    actor = 'pitrou'
    assignee = 'none'
    closed = True
    closed_date = <Date 2009-01-21.01:06:34.907>
    closer = 'pitrou'
    components = []
    creation = <Date 2009-01-20.00:49:29.627>
    creator = 'vstinner'
    dependencies = []
    files = ['12820']
    hgrepos = []
    issue_num = 5008
    keywords = ['patch']
    message_count = 9.0
    messages = ['80230', '80231', '80248', '80294', '80296', '80297', '80298', '80300', '80305']
    nosy_count = 2.0
    nosy_names = ['pitrou', 'vstinner']
    pr_nums = []
    priority = 'release blocker'
    resolution = 'fixed'
    stage = None
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue5008'
    versions = ['Python 3.0', 'Python 3.1']

    @vstinner
    Copy link
    Member Author

    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.

    @pitrou pitrou added release-blocker type-bug An unexpected behavior, bug, or error labels Jan 20, 2009
    @vstinner
    Copy link
    Member Author

    Patch _including a test_:

    + if (append)
    + lseek(self->fd, 0, SEEK_END);

    @pitrou
    Copy link
    Member

    pitrou commented Jan 20, 2009

    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)

    @vstinner
    Copy link
    Member Author

    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 :-)

    @pitrou
    Copy link
    Member

    pitrou commented Jan 20, 2009

    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

    @pitrou
    Copy link
    Member

    pitrou commented Jan 20, 2009

    Last thing, in your patch there is a forward declaration to
    portable_lseek but it doesn't look used anywhere...

    @vstinner
    Copy link
    Member Author

    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 bpo-5016.

    @vstinner
    Copy link
    Member Author

    Version 4: ok, let's use *portable*_lseek() instead of the "ugly"
    lseek() function (not compatible with large files on Windows).

    @pitrou
    Copy link
    Member

    pitrou commented Jan 21, 2009

    Committed in r68835, r68836, r68837, r68838.

    @pitrou pitrou closed this as completed Jan 21, 2009
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    release-blocker type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants