8000 VFS Module does not reserve file descriptors 0, 1, and 2 · Issue #8309 · RIOT-OS/RIOT · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

VFS Module does not reserve file descriptors 0, 1, and 2 #8309

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.

Sign up for GitHub

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

Jump to bottom
Closed
samkumar opened this issue Dec 20, 2017 · 5 comments
Closed

VFS Module does not reserve file descriptors 0, 1, and 2 #8309

samkumar opened this issue Dec 20, 2017 · 5 comments
Labels
Area: fs Area: File systems Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)

Comments

@samkumar
Copy link
Contributor

Description

The VFS module does not reserve file descriptors 0, 1, and 2 for stdin, stdout, and stderr. As a result, the following bugs exist:

  • Opening a file could be assigned to one of these descriptors.
  • Utilities in newlib such as printf do not work. The output is written to file descriptor 1 and uart_stdio_write never gets called.

More generally, there seems to be no vfs_init procedure, so it seems that all the _vfs_open_files array is unintialized. In my opinion, the correct solution is to add a vfs_init procedure that initializes the _vfs_open_files array and reserves file descriptors 0, 1, and 2.

Steps to reproduce the issue

Method 1: Call printf().
Method 2: Open a socket with the posix_sockets module.

Expected results

Expected Results for Method 1: The output is written to the screen.
Expected Results for Method 2: The resulting file descriptor is greater than 2.

Actual results

Actual Results for Method 1: Nothing is written to the screen.
Actual Results for Method 2: The resulting file descriptor is 0.

Versions

I am building the code from a Linux computer running Ubuntu 16.04.
The platform I am targeting is HAMILTON (@immesys).

Output from print_toolchain_versions.sh:

./print_toolchain_versions.sh 
Installed toolchain versions
----------------------------
          native gcc: gcc (Ubuntu 5.4.0-6ubuntu1~16.04.5) 5.4.0 20160609
          msp430-gcc: msp430-gcc (GCC) 4.6.3 20120301 (mspgcc LTS 20120406 unpatched)
             avr-gcc: missing/error
   arm-none-eabi-gcc: arm-none-eabi-gcc (15:4.9.3+svn231177-1) 4.9.3 20150529 (prerelease)
    mips-mti-elf-gcc: missing/error
               clang: clang version 3.8.0-2ubuntu4 (tags/RELEASE_380/final)
arm-none-eabi-newlib: "2.2.0"
 mips-mti-elf-newlib: missing/error
            avr-libc: missing/error (missing/error)
            cppcheck: missing
          coccinelle: missing
@immesys
Copy link
Contributor
immesys commented Dec 20, 2017

This would affect other platforms too though right?

@samkumar
Copy link
Contributor Author

Yes, but after further investigation I believe that the problem may be restricted to rtt_stdio. The uart_stdio module registers stdin, stdout, and stderr with VFS in its init function.

So there are two options that I have identified.

  1. Copy the same code to rtt_stdio.
  2. Move the code into the VFS module.

Personally, I am in favor of the second option, since otherwise there would be multiple copies of the code to maintain. Plus, I think there needs to be a vfs_init function, at the very least to zero out the _vfs_open_files array before it is used.

@samkumar samkumar mentioned this issue Dec 21, 2017
Closed
@samkumar
Copy link
Contributor Author

I've written a fix for this. See Pull Request #8310.

@PeterKietzmann PeterKietzmann added the Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) label Jan 2, 2018
jnohlgard pushed a commit to jnohlgard/RIOT that referenced this issue Feb 13, 2018
sys/vfs: Exclude stdio file numbers from auto allocation
be14d2e 
Fixes RIOT-OS#8309 (RIOT-OS#8309)
jnohlgard pushed a commit to jnohlgard/RIOT that referenced this issue Feb 13, 2018
unittests/vfs: Add test case for RIOT-OS#8309
d24cd4f
@jnohlgard jnohlgard mentioned this issue Feb 13, 2018
Merged
@jnohlgard
Copy link
Member

See an alternative solution in #8552, which does not require adding any new functions or dependencies between vfs and other modules.

@jnohlgard jnohlgard added the Area: fs Area: File systems label Feb 19, 2018
@samkumar samkumar mentioned this issue Feb 28, 2018
Merged
@samkumar
Copy link
Contributor Author

@gebart, thanks for your comments on this issue and in my Pull Request #8310.

I agree that your solution in #8552 is cleaner, in the sense that VFS doesn't depend on other modules. However, it doesn't resolve that problem that printf() does not work in rtt_stdio (see Method #1 in the original description). The reason is that rtt_stdio doesn't bind the file descriptors 0, 1, and 2 if VFS is used, yet printf() writes to file descriptor 1 (stdout) when VFS is used.

Basically, if you look at the comment I made above, my solution uses Option 2, and yours uses Option 1. But to make Option 1 work, we also need to duplicate the binding logic used by uart_stdio, in rtt_stdio. I've opened Pull Request #8654 that does just that.

In essence, your solution in #8552 fixes the first half of the problem, that VFS could bind file descriptors 0, 1, and 2 to other files, causing printf() to be redirected to those files. But we also need #8654 to fix the second half of the problem, which is that rtt_stdio needs to allocate those file descriptors so that printf() works.

panail pushed a commit to panail/RIOT that referenced this issue Oct 29, 2018
sys/vfs: Exclude stdio file numbers from auto allocation
d3a0d22 
Fixes RIOT-OS#8309 (RIOT-OS#8309)
panail pushed a commit to panail/RIOT that referenced this issue Oct 29, 2018
unittests/vfs: Add test case for RIOT-OS#8309
497dff2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Area: fs Area: File systems Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

sys/vfs: reserve file descriptors for stdin, stdout, and stderr samkumar/RIOT-OS
4 participants
@jnohlgard @immesys @samkumar @PeterKietzmann
0