Superfluous call to isatty in open() when buffering >= 0

[Dav1dde]

BPO	34070
Nosy	@vstinner, @Dav1dde
PRs	bpo-34070: only check for isatty when buffering < 0 #8187

^{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 2018-10-19.22:34:22.048>
created_at = <Date 2018-07-08.18:12:53.330>
labels = ['3.7', '3.8', 'expert-IO', 'performance']
title = 'Superfluous call to isatty in open() when buffering >= 0'
updated_at = <Date 2018-10-19.22:34:22.046>
user = 'https://github.com/Dav1dde'

bugs.python.org fields:

activity = <Date 2018-10-19.22:34:22.046>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2018-10-19.22:34:22.048>
closer = 'vstinner'
components = ['IO']
creation = <Date 2018-07-08.18:12:53.330>
creator = 'Dav1d'
dependencies = []
files = []
hgrepos = []
issue_num = 34070
keywords = ['patch']
message_count = 3.0
messages = ['321281', '328066', '328067']
nosy_count = 2.0
nosy_names = ['vstinner', 'Dav1d']
pr_nums = ['8187']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'performance'
url = 'https://bugs.python.org/issue34070'
versions = ['Python 3.6', 'Python 3.7', 'Python 3.8']

[Dav1dde]

_iomodule.c:_io_open_impl checks for isatty even if not necessary (when buffering >= 0).

Code:

cpython/Modules/_io/_iomodule.c

Line 392 in c0ee341

PyObject *res = _PyObject_CallMethodId(raw, &PyId_isatty, NULL);

    {
        PyObject *res = _PyObject_CallMethodId(raw, &PyId_isatty, NULL);
        if (res == NULL)
            goto error;
        isatty = PyLong_AsLong(res);
        Py_DECREF(res);
        if (isatty == -1 && PyErr_Occurred())
            goto error;
    }

    if (buffering == 1 || (buffering < 0 && isatty)) {
        buffering = -1;
        line_buffering = 1;
    }
    else
        line_buffering = 0;

Python Code to reproduce:

with open('foo', 'rb', buffering=0) as f:
    f.read()

This generates an error (can be seen with strace):

ioctl(5, TCGETS, 0x7ffef1435b60) = -1 ENOTTY (Inappropriate ioctl for device)

I'll open a PR shortly.

[vstinner]

New changeset 8deab96 by Victor Stinner (David Herberth) in branch 'master':
bpo-34070: open() only checks for isatty if buffering < 0 (GH-8187)
8deab96

[vstinner]

David Herberth: Thanks for report the issue, I didn't noticed it previously even if I read strace output frequently! And thanks for the fix.

This generates an error (can be seen with strace):
ioctl(5, TCGETS, 0x7ffef1435b60) = -1 ENOTTY (Inappropriate ioctl for device)

Honestly, I don't think that it's a major bug, so I suggest to not backport the change to 3.7 and older. I prefer to not change stable changes to avoid any risk of regression.