Skip to content

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

Closed
Closed
_pyio checks that os.name == 'win32' instead of 'nt'#69069
Assignees
serhiy-storchaka
Labels
OS-windowstopic-IOtype-bugAn unexpected behavior, bug, or error

Description

@CosimoLupo
CosimoLupo
mannequin
opened on Aug 17, 2015
BPO 24881
Nosy @pfmoore, @vstinner, @tjguk, @4kir4, @embray, @zware, @serhiy-storchaka, @eryksun, @zooba
Files
  • pyio_setmode.diff: call setmode(fd, O_BINARY) if sys.platform in {'win32', 'cygwin'} like in C implementation
    		•

    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 = 'https://github.com/serhiy-storchaka'
closed_at = <Date 2015-08-28.19:22:18.911>
created_at = <Date 2015-08-17.14:11:07.420>
labels = ['type-bug', 'expert-IO', 'OS-windows']
title = "_pyio checks that `os.name == 'win32'` instead of 'nt'"
updated_at = <Date 2016-10-17.10:37:42.399>
user = 'https://bugs.python.org/CosimoLupo'

    bugs.python.org fields:

    activity = <Date 2016-10-17.10:37:42.399>
actor = 'vstinner'
assignee = 'serhiy.storchaka'
closed = True
closed_date = <Date 2015-08-28.19:22:18.911>
closer = 'serhiy.storchaka'
components = ['Windows', 'IO']
creation = <Date 2015-08-17.14:11:07.420>
creator = 'Cosimo Lupo'
dependencies = []
files = ['40222']
hgrepos = []
issue_num = 24881
keywords = ['patch']
message_count = 7.0
messages = ['248728', '248978', '248986', '249290', '249291', '278800', '278801']
nosy_count = 11.0
nosy_names = ['paul.moore', 'vstinner', 'tim.golden', 'akira', 'python-dev', 'erik.bray', 'zach.ware', 'serhiy.storchaka', 'eryksun', 'steve.dower', 'Cosimo Lupo']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue24881'
versions = ['Python 3.5', 'Python 3.6']

    Activity

    1. CosimoLupo commented on Aug 17, 2015

      @CosimoLupo
      CosimoLupomannequin
      on Aug 17, 2015
      MannequinAuthor

      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 error
      on Aug 17, 2015
    3. zware
      added
      OS-windows
      on Aug 17, 2015

    4. 4kir4 commented on Aug 21, 2015

      @4kir4
      4kir4mannequin
      on Aug 21, 2015
      Mannequin

      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
      eryksun
      on Aug 22, 2015
      Contributor

      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-dev
      python-devmannequin
      on Aug 28, 2015
      Mannequin

      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
      serhiy-storchaka
      on Aug 28, 2015
      Member

      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
      embray
      on Oct 17, 2016
      Contributor

      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
      vstinner
      on Oct 17, 2016
      Member

      "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
      vstinner
      on May 15, 2026
      Member

      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.

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

    Metadata

    Metadata

    Assignees

    Labels

    OS-windowstopic-IOtype-bugAn unexpected behavior, bug, or error

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions