Issue 24881: _pyio checks that `os.name == 'win32'` instead of 'nt'

Messages (7) msg248728 - (view) Author: Cosimo Lupo (Cosimo Lupo) Date: 2015-08-17 14:11

the `_pyio` module at line 16 tries to check whether it is running on Windows platform, by doing:

```
if os.name == 'win32':
    from msvcrt import setmode as _setmode
else:
    _setmode = None
```

However, the string returned by os.name is 'nt' and not 'win32' (the latter is returned by `sys.platform`). Therefore, the value is always False and the setmode function from mscvrt module is never imported.

Thank you.
Cheers,

Cosimo

msg248978 - (view) Author: Akira Li (akira) * Date: 2015-08-21 20:18

To make _pyio correspond to the C version I've added 

  sys.platform in {'win32', 'cygwin'} 

condition. See the attached pyio_setmode.diff 

It is not clear why the absence of _setmode(fd, os.O_BINARY) is not detected by tests.

(a) a corresponding test should be added
(b) OR _setmode() call should be removed from both Python and C io versions

msg248986 - (view) Author: Eryk Sun (eryksun) * Date: 2015-08-22 03:01

> It is not clear why the absence of _setmode(fd, os.O_BINARY)
> is not detected by tests.

It's only a problem when an existing text-mode file descriptor is passed in. For example, in text mode the CRT handles b'\x1a' as an EOF marker:

    Python 3.5.0b4 (v3.5.0b4:c0d641054635, Jul 26 2015, 07:11:12)
    [MSC v.1900 64 bit (AMD64)] on win32
    Type "help", "copyright", "credits" or "license" for more information.
    >>> import os, _pyio
    >>> _ = open('testfile', 'wb').write(b'abc\x1adef')

    >>> fd = os.open('testfile', os.O_RDONLY|os.O_TEXT)
    >>> f = _pyio.open(fd, 'rb')
    >>> f.read()
    b'abc'
    >>> f.close()

    >>> f = _pyio.open('testfile', 'rb')
    >>> f.read()
    b'abc\x1adef'

The setmode call ensures a file descriptor is in the expected binary mode:

    >>> import msvcrt
    >>> fd = os.open('testfile', os.O_RDONLY|os.O_TEXT)
    >>> old_mode = msvcrt.setmode(fd, os.O_BINARY)
    >>> old_mode == os.O_TEXT
    True
    >>> f = _pyio.open(fd, 'rb')
    >>> f.read()
    b'abc\x1adef'

msg249290 - (view) Author: Roundup Robot (python-dev) Date: 2015-08-28 19:20

New changeset 687da8760a58 by Serhiy Storchaka in branch '3.5':
Issue #24881: Fixed setting binary mode in Python implementation of FileIO
https://hg.python.org/cpython/rev/687da8760a58

New changeset 2dd9294f679d by Serhiy Storchaka in branch 'default':
Issue #24881: Fixed setting binary mode in Python implementation of FileIO
https://hg.python.org/cpython/rev/2dd9294f679d

msg249291 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * Date: 2015-08-28 19:22

Good catch. Thank you for your report Cosimo. Thank you for your patch Akira.

msg278800 - (view) Author: Erik Bray (erik.bray) * Date: 2016-10-17 10:26

FWIW this patch broke the _pyio module on Cygwin, as the msvcrt module is not built on Cygwin.  AFAICT this is only a problem for Python built with MSVCRT, which Cygwin does not use.  When test case works as expected on Cygwin without this.

msg278801 - (view) Author: STINNER Victor (vstinner) * Date: 2016-10-17 10:37

"FWIW this patch broke the _pyio module on Cygwin,"

Please open a new issue, this one is closed.

History Date User Action Args 2016-10-17 10:37:42vstinnersetmessages: + msg278801 2016-10-17 10:26:17erik.braysetnosy: + erik.bray
messages: + msg278800
2015-08-28 19:22:18serhiy.storchakasetstatus: open -> closed
resolution: fixed
messages: + msg249291

stage: patch review -> resolved

2015-08-28 19:20:53python-devsetnosy: + python-dev
messages: + msg249290
2015-08-28 19:03:15serhiy.storchakasetassignee: serhiy.storchaka
stage: patch review 2015-08-22 03:01:07eryksunsetnosy: + eryksun
messages: + msg248986
2015-08-21 20:55:51serhiy.storchakasetnosy: + vstinner, serhiy.storchaka

versions: - Python 3.4

2015-08-21 20:18:40akirasetfiles: + pyio_setmode.diff

nosy: + akira
messages: + msg248978

keywords: + patch

2015-08-17 14:43:18zach.waresetnosy: + paul.moore, tim.golden, zach.ware, steve.dower

components: + Windows
versions: + Python 3.4, Python 3.6

2015-08-17 14:11:07Cosimo Lupocreate