_pyio checks that os.name == 'win32' instead of 'nt' #69069

Activity

1. CosimoLupo commented on Aug 17, 2015

   

   CosimoLupomannequin

   on Aug 17, 2015

   MannequinAuthor

   More actions

   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
2. CosimoLupo

   added

   topic-IO

   type-bugAn unexpected behavior, bug, or errorAn unexpected behavior, bug, or error

   on Aug 17, 2015
3. zware

   added

   OS-windows

   on Aug 17, 2015
4. 4kir4 commented on Aug 21, 2015

   

   4kir4mannequin

   on Aug 21, 2015

   Mannequin

   More actions

   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
5. eryksun commented on Aug 22, 2015

   

   eryksun

   on Aug 22, 2015

   Contributor

   More actions

   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'
6. serhiy-storchaka

   self-assigned this

   on Aug 28, 2015
7. python-dev commented on Aug 28, 2015

   

   python-devmannequin

   on Aug 28, 2015

   Mannequin

   More actions

   New changeset 687da8760a58 by Serhiy Storchaka in branch '3.5':
   Issue bpo-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 bpo-24881: Fixed setting binary mode in Python implementation of FileIO
   https://hg.python.org/cpython/rev/2dd9294f679d
8. serhiy-storchaka commented on Aug 28, 2015

   

   serhiy-storchaka

   on Aug 28, 2015

   Member

   More actions

   Good catch. Thank you for your report Cosimo. Thank you for your patch Akira.
9. serhiy-storchaka

   closed this as completedon Aug 28, 2015
10. embray commented on Oct 17, 2016

    

    embray

    on Oct 17, 2016

    Contributor

    More actions

    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.
11. vstinner commented on Oct 17, 2016

    

    vstinner

    on Oct 17, 2016

    Member

    More actions

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

    Please open a new issue, this one is closed.
12. ezio-melotti

    transferred this issue fromon Apr 10, 2022
13. vstinner commented on May 15, 2026

    

    vstinner

    on May 15, 2026

    Member

    More actions

    FWIW this patch broke the _pyio module on Cygwin, as the msvcrt module is not built on Cygwin.

    I wrote PR gh-149899 to fix the regression.