Skip to content

wasi: allow WASI stdio to be configured#33544

Closed
cjihrig wants to merge 1 commit intonodejs:masterfrom
cjihrig:wasi-stdio
Closed

wasi: allow WASI stdio to be configured#33544
cjihrig wants to merge 1 commit intonodejs:masterfrom
cjihrig:wasi-stdio

Conversation

@cjihrig
Copy link
Contributor

@cjihrig cjihrig commented May 24, 2020

This commit adds stdin, stderr, and stdout options to WASI, which allow the stdio streams to be configured.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

@cjihrig
wasi: allow WASI stdio to be configured
e3d50e5 
This commit adds stdin, stderr, and stdout options to WASI, which
allow the stdio streams to be configured.
@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. wasi Issues and PRs related to the WebAssembly System Interface. labels May 24, 2020
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented May 24, 2020 •
edited by cjihrig
Loading

CI: https://ci.nodejs.org/job/node-test-pull-request/31541/ 💛

@cjihrig cjihrig added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label May 24, 2020
bnoordhuis
bnoordhuis approved these changes May 25, 2020
View reviewed changes
Copy link
Member

@bnoordhuis bnoordhuis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM but what happens when I pass e.g. { stdin: -42 }? validateInt32() won't reject it.

@cjihrig
Copy link
Contributor Author

cjihrig commented May 25, 2020

LGTM but what happens when I pass e.g. { stdin: -42 }? validateInt32() won't reject it.

The third argument to validateInt32(stdin, 'options.stdin', 0); causes negative values to be rejected. Invalid file descriptors in general throw an exception: Error: UVWASI_EBADF, uvwasi_init

BridgeAR pushed a commit to BridgeAR/node that referenced this pull request May 30, 2020
@cjihrig @BridgeAR
wasi: allow WASI stdio to be configured
e3b54b4 
This commit adds stdin, stderr, and stdout options to WASI, which
allow the stdio streams to be configured.

PR-URL: nodejs#33544
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: Zeyu Yang <himself65@outlook.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
@BridgeAR
Copy link
Member

BridgeAR commented May 30, 2020

Landed in e3b54b4

@BridgeAR BridgeAR closed this May 30, 2020
@cjihrig cjihrig deleted the wasi-stdio branch May 30, 2020 16:40
@cjihrig cjihrig mentioned this pull request May 30, 2020
Closed
codebytere pushed a commit that referenced this pull request Jun 18, 2020
@cjihrig @codebytere
wasi: allow WASI stdio to be configured
af14c1f 
This commit adds stdin, stderr, and stdout options to WASI, which
allow the stdio streams to be configured.

PR-URL: #33544
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: Zeyu Yang <himself65@outlook.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
@codebytere codebytere mentioned this pull request Jun 28, 2020
Merged
codebytere pushed a commit that referenced this pull request Jun 30, 2020
@cjihrig @codebytere
wasi: allow WASI stdio to be configured
e3cdc03 
This commit adds stdin, stderr, and stdout options to WASI, which
allow the stdio streams to be configured.

PR-URL: #33544
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: Zeyu Yang <himself65@outlook.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
codebytere pushed a commit that referenced this pull request Jul 9, 2020
@cjihrig @codebytere
wasi: allow WASI stdio to be configured
8adfb54 
This commit adds stdin, stderr, and stdout options to WASI, which
allow the stdio streams to be configured.

PR-URL: #33544
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: Zeyu Yang <himself65@outlook.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
@codebytere codebytere mentioned this pull request Jul 13, 2020
Merged
This was referenced Jan 25, 2022
Open
Open
Open
Open
Open
Open
Open
This was referenced Jan 26, 2022
Open
Open
@snyk-bot snyk-bot mentioned this pull request Sep 27, 2022
Open
@wedataintelligence wedataintelligence mentioned this pull request Sep 27, 2022
Open
This was referenced Sep 27, 2022
Open
Open
@137717unity 137717unity mentioned this pull request Sep 28, 2022
Open
@snyk-bot snyk-bot mentioned this pull request Sep 28, 2022
Open
@ron-a20 ron-a20 mentioned this pull request Sep 28, 2022
Open
This was referenced Sep 28, 2022
Open
Open
@137717unity 137717unity mentioned this pull request Jun 23, 2023
Open
@molllyn1 molllyn1 mentioned this pull request Jun 23, 2023
Open
@aliceUnhinged613 aliceUnhinged613 mentioned this pull request Jun 24, 2023
Open
@ron-a20 ron-a20 mentioned this pull request Jun 24, 2023
Open
@wedataintelligence wedataintelligence mentioned this pull request Jun 24, 2023
Open
@aliceUnhinged613 aliceUnhinged613 mentioned this pull request Nov 25, 2023
Open
@wedataintelligence wedataintelligence mentioned this pull request Nov 28, 2023
Open
@baby636 baby636 mentioned this pull request Dec 7, 2023
Open
This was referenced Dec 15, 2023
Open
Open
@baby636 baby636 mentioned this pull request Jan 6, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bnoordhuis bnoordhuis bnoordhuis approved these changes

@jasnell jasnell jasnell approved these changes

@addaleax addaleax addaleax approved these changes

@himself65 himself65 himself65 approved these changes

@juanarbol juanarbol juanarbol approved these changes

+1 more reviewer

@devnexen devnexen devnexen approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

author ready PRs that have at least one approval, no pending requests for changes, and a CI started. c++ Issues and PRs that require attention from people who are familiar with C++. wasi Issues and PRs related to the WebAssembly System Interface.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

9 participants

@cjihrig @nodejs-github-bot @BridgeAR @bnoordhuis @jasnell @addaleax @devnexen @himself65 @juanarbol