Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

interaction of _read on fd 0 in terminal #16299

Closed
spk121 opened this issue Nov 12, 2023 · 5 comments · Fixed by #16313
Closed

interaction of _read on fd 0 in terminal #16299

spk121 opened this issue Nov 12, 2023 · 5 comments · Fixed by #16313
Assignees
Labels
Area-Input Related to input processing (key presses, mouse, etc.) Area-Server Down in the muck of API call servicing, interprocess communication, eventing, etc. In-PR This issue has a related PR Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Tag-Fix Doesn't match tag requirements

Comments

@spk121
Copy link

spk121 commented Nov 12, 2023

Windows Terminal version

1.19.2682.0

Windows build number

Version 10.0.22621.2428

Other Software

No response

Steps to reproduce

Compile the attached, run, and press enter multiple times. Behavior is the same with MSVC in Visual studio 2022 and with latest MinGW with GCC using 64-bit UCRT.

#include <io.h>
#include <stdio.h>
#include <errno.h>


int main()
{
  int fd = 0;
  printf("fd = %d\n", fd);

  int ret;
  char buf[128];

  while (1)
    {
      errno = 0;
      ret = _read (fd, buf, 1);
      printf("ret = %d, errno = %d, char = %u\n", ret, errno, buf[0]);
    }
  return 0;
}

Expected Behavior

I expected the behavior found on MinGW's mintty, when I press the enter key multiple times.

fd = 0

ret = 1, errno = 0, char = 10

ret = 1, errno = 0, char = 10

ret = 1, errno = 0, char = 10

ret = 1, errno = 0, char = 10

Actual Behavior

This is what happens in Terminal when I press the enter key multiple times on a recent Terminal

fd = 0


ret = 1, errno = 0, char = 13
ret = -1, errno = 13, char = 13


ret = 1, errno = 0, char = 13
ret = -1, errno = 13, char = 13


ret = 1, errno = 0, char = 13
ret = -1, errno = 13, char = 13


ret = 1, errno = 0, char = 13
ret = -1, errno = 13, char = 13
@spk121 spk121 added Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Nov 12, 2023
@spk121
Copy link
Author

spk121 commented Nov 12, 2023

Apparently something having to do with handling of CR LF line endings in terminal, but, that _read errors with EACCES is unexpected.

@zadjii-msft
Copy link
Member

I'm gonna reckon that this maybe regressed due to #15783. I'll tag @lhecker to investigate. Thanks for the minimal repro!

@zadjii-msft zadjii-msft added this to the Terminal v1.20 milestone Nov 13, 2023
@zadjii-msft zadjii-msft added Area-Input Related to input processing (key presses, mouse, etc.) Area-Server Down in the muck of API call servicing, interprocess communication, eventing, etc. labels Nov 13, 2023
@zadjii-msft
Copy link
Member

maybe #15662?

@spk121
Copy link
Author

spk121 commented Nov 14, 2023

I noticed that if you change the _read call to _read(fd, buf, 2) it works fine and always returns 1 character of ASCII 10.. It is like the _read needs a two character buffer to hold (i presume) "\r\n" before some sort of cooked logic turns it into "\n" before storing it into buf.

@lhecker
Copy link
Member

lhecker commented Nov 15, 2023

I believe this is a duplicate of #16223. I haven't checked how _read() is implemented but I bet it's a wrapper around ReadFile()...

@microsoft-github-policy-service microsoft-github-policy-service bot added the In-PR This issue has a related PR label Nov 15, 2023
@carlos-zamora carlos-zamora removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Nov 15, 2023
@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs-Tag-Fix Doesn't match tag requirements label Nov 15, 2023
microsoft-github-policy-service bot pushed a commit that referenced this issue Nov 21, 2023
This fixes an issue where character-wise reading of an input like "abc"
would return "a" to the caller, store "b" as a partial translation
(= wrong) and return "c" for the caller to store it for the next call.

Closes #16223
Closes #16299

## Validation Steps Performed
* `ReadFile` with a buffer size of 1 returns inputs character by
  character without dropping any inputs ✅

---------

Co-authored-by: Dustin L. Howett <duhowett@microsoft.com>
DHowett added a commit that referenced this issue Dec 4, 2023
This fixes an issue where character-wise reading of an input like "abc"
would return "a" to the caller, store "b" as a partial translation
(= wrong) and return "c" for the caller to store it for the next call.

Closes #16223
Closes #16299

## Validation Steps Performed
* `ReadFile` with a buffer size of 1 returns inputs character by
  character without dropping any inputs ✅

---------

Co-authored-by: Dustin L. Howett <duhowett@microsoft.com>
(cherry picked from commit 63b3820)
Service-Card-Id: 91122022
Service-Version: 1.19
DHowett added a commit that referenced this issue Jan 23, 2024
This fixes an issue where character-wise reading of an input like "abc"
would return "a" to the caller, store "b" as a partial translation
(= wrong) and return "c" for the caller to store it for the next call.

Closes #16223
Closes #16299

## Validation Steps Performed
* `ReadFile` with a buffer size of 1 returns inputs character by
  character without dropping any inputs ✅

---------

Co-authored-by: Dustin L. Howett <duhowett@microsoft.com>
(cherry picked from commit 63b3820)
Service-Card-Id: 91108808
Service-Version: 1.18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Input Related to input processing (key presses, mouse, etc.) Area-Server Down in the muck of API call servicing, interprocess communication, eventing, etc. In-PR This issue has a related PR Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Tag-Fix Doesn't match tag requirements
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants