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

stdio_native: initial import #12981

Merged
merged 2 commits into from
Dec 19, 2019
Merged

stdio_native: initial import #12981

merged 2 commits into from
Dec 19, 2019

Conversation

miri64
Copy link
Member

@miri64 miri64 commented Dec 18, 2019

Contribution description

This provides a stdio module for native, so the abstraction of stdio is the same as on other boards.

While implementing tests for #12975, I noticed, that on native when using vfs the fmt print functions would not work. This is because the fmt module uses the write() system call which is bend by the native_vfs module (or the newlib_syscalls_default module for real boards) to use vfs_write(). However, since native doesn't use a stdio module without this PR to print, but just writes to the hosts standard I/O directly, the STDIN, STDOUT, and STDERR are never initialized for vfs, vfs fails here:

RIOT/sys/vfs/vfs.c

Lines 320 to 323 in 33b1f9e

int res = _fd_is_valid(fd);
if (res < 0) {
return res;
}

Testing procedure

Without stdio_native (revert all commits related to it; the HEAD when opening), the provided test application tests/vfs_and_stdio will fail on native:

BOARD=native make -C tests/vfs_plus_stdio flash test

With stdio_native available it will succeed.

BOARD=native make -C examples/hello-world all term

and

BOARD=native make -C tests/shell all test

should also still work.

Issues/PRs references

None

@miri64 miri64 added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Platform: native Platform: This PR/issue effects the native platform Type: new feature The issue requests / The PR implemements a new feature for RIOT labels Dec 18, 2019
@miri64 miri64 added this to the Release 2020.01 milestone Dec 18, 2019
@miri64 miri64 added Area: cpu Area: CPU/MCU ports CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR CI: run tests If set, CI server will run tests on hardware for the labeled PR labels Dec 18, 2019
@miri64
Copy link
Member Author

miri64 commented Dec 18, 2019

Forgot to push the test 😅

Copy link
Member

@SemjonWilke SemjonWilke left a comment

Choose a reason for hiding this comment

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

ACK
test works as explained
PR is very reasonable
code looks good

On `native` when using `vfs` the `fmt` print functions do not work.
This is because the `fmt` module uses the `write()` system call which
is bend by the `native_vfs` module to use `vfs_write()`. However,
`native` does not use a `stdio` module to print. Instead, it just
writes to the hosts standard I/O directly. As such, STDIN, STDOUT, and
STDERR are never initialized for `vfs` so `vfs` does not recognize
`STDIN_FILENO`, `STDOUT_FILENO` and `STDERR_FILENO` as valid file
descriptors.

This test case showcases this bug.
@miri64
Copy link
Member Author

miri64 commented Dec 19, 2019

@aabadie Can we merge?

@miri64 miri64 merged commit 501e700 into RIOT-OS:master Dec 19, 2019
@miri64 miri64 deleted the native/feat/stdio branch December 19, 2019 14:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: cpu Area: CPU/MCU ports CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR CI: run tests If set, CI server will run tests on hardware for the labeled PR Platform: native Platform: This PR/issue effects the native platform Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Type: new feature The issue requests / The PR implemements a new feature for RIOT
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants