Skip to content

core:os package read_entire_file_xxx procedures don't work on /proc filesystem even if on a Linux system #4264

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

Open
edyu opened this issue Sep 18, 2024 · 4 comments

Comments

@edyu
Copy link
Contributor

edyu commented Sep 18, 2024

Context

Please provide any relevant information about your setup. This is important in case the issue is not reproducible except for under certain conditions.

        Odin:    dev-2024-09:8814170ed
        OS:      Ubuntu 24.04.1 LTS, Linux 5.15.153.1-microsoft-standard-WSL2
        CPU:     AMD Ryzen 7 5800HS with Radeon Graphics
        RAM:     11662 MiB
        Backend: LLVM 18.1.3

Expected Behavior

os.read_entire_file and fellow procedures should be able to read /proc files such as /proc/meminfo.

Current Behavior

os.read_entire_file procedures don't return error but will only return an empty slice.

Failure Information (for bugs)

Please help provide information about the failure if this is a bug. If it is not a bug, please remove the rest of this template.

Steps to Reproduce

Please provide detailed steps for reproducing the issue.

    data := os.read_entire_file_or_err("/proc/meminfo) or_return
    defer delete(data)
    fmt.println(data)
@edyu edyu changed the title core:os package read_entire_file_xxx procedures doesn't work on /proc filesystem even if I'm on a Linux system core:os package read_entire_file_xxx procedures don't work on /proc filesystem even if on a Linux system Sep 18, 2024
@flysand7
Copy link
Contributor

Some relevant info about the bug:

The read_entire_file* calls in question go through the read_entire_file_from_handle_or_err procedure, which first uses stat() to get the length of the file, then attempts to read the file of that length with a single read call.

Odin/core/os/os.odin

Lines 144 to 147 in 8814170

length := file_size(fd) or_return
if length <= 0 {
return nil, nil
}

The issue is that 0-sized files can contain information too, seems like that's a quirk of the /proc filesystem on linux. The stat command shows that the file is zero-sized, and is a regular file:

flysand@DESKTOP-PMKHH3T:/mnt/c/Users/flysand7/Odin$ stat /proc/meminfo
  File: /proc/meminfo
  Size: 0               Blocks: 0          IO Block: 1024   regular empty file
Device: 3fh/63d Inode: 4026532019  Links: 1
Access: (0444/-r--r--r--)  Uid: (    0/    root)   Gid: (    0/    root)
Access: 2024-09-18 18:50:33.668122843 +1100
Modify: 2024-09-18 18:50:33.668122843 +1100
Change: 2024-09-18 18:50:33.668122843 +1100
 Birth: -

Which seems to suggest that reading a file by first asking for its length is wrong on linux. At least with input streams (/dev/stdin etc), you can tell that can't read "file size" bytes and get the whole file back, because they are block/character devices, pipes or otherwise.

Prompting @laytan, @Yawning to join this discussion, as I don't think it's a trivial fix.

Since from what I know we can't tell these files from other regular files (aside from the name, which is unreliable, because we might not have that), my suggestion for the fix is making read_entire_file always read files in chunks, size of which will be controlled by a parameter with a nice default value.

@flysand7
Copy link
Contributor

A complete example:

package bug

import "core:fmt"
import "core:os"

main :: proc() {
	data, err := os.read_entire_file_or_err("/proc/meminfo")
	fmt.println(data, err)
}

@Yawning
Copy link
Contributor

Yawning commented Sep 20, 2024

Since from what I know we can't tell these files from other regular files

Use fstatfs

@laytan
Copy link
Collaborator

laytan commented Oct 8, 2024

Isn't this fixed in os2? I think we stat there and if it returns size 0 we read into a buffer in chunks instead of all at once. May want to do that here too.

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

No branches or pull requests

4 participants