Skip to content

gh-159677: fix FileIO.seekable incorrectly returning false - #159678

Draft
KowalskiThomas wants to merge 3 commits into
python:mainfrom
KowalskiThomas:kowalski/fix-fileio-seekable-incorrectly-returning-false
Draft

KowalskiThomas wants to merge 3 commits into
python:mainfrom
KowalskiThomas:kowalski/fix-fileio-seekable-incorrectly-returning-false

Conversation

@KowalskiThomas

Copy link
Copy Markdown
Contributor

Fixes gh-159677.

@KowalskiThomas

Copy link
Copy Markdown
Contributor Author

Seems like the Docs check has flaked :|

Comment thread Misc/NEWS.d/next/Library/2026-10-18-17-26-00.gh-issue-159677.fIoSk1.rst Outdated
Comment thread Modules/_io/fileio.c
Py_END_ALLOW_THREADS

if (self->seekable < 0) {
if (self->seekable < 0 && (res >= 0 || errno == ESPIPE || posobj == NULL)) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure across the set of OS + kernel + version which Python runs on this is correct... Can work on evaluating that thoroughly but my leaning would be just making it so specific returns we know about that indicate "bad argument" pass through.

For instance EBADF should probably still mark seekable as False / I could see code relying on that. Something like: EINVAL, ENXIO, EOVERFLOW for Linux; EBADF and EINVAL for OpenBSD.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea here was that only in the following cases we're 100% sure we know for sure what seekable should be:

  • res >= 0 i.e. it succeeded
  • errno == ESPIPE i.e. Illegal seek
  • posobj == NULL i.e. offset == 0 in which case a failure would indicate a seek for SEEK_CUR/SEEK_END failed (so the problem is the file itself) => it's OK to cache the result of seeking directly

If we don't set the cache, "someone else" hopefully will later, but at least we're sure we never store a false value here.

Does that sound reasonable?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

FileIO.seekable() is incorrect if determined by an invalid seek call

2 participants