Skip to content

Fix nob_read_entire_file for procfs files and pipes#261

Open
NeikiDev wants to merge 1 commit into
tsoding:mainfrom
NeikiDev:fix-242-read-entire-file
Open

Fix nob_read_entire_file for procfs files and pipes#261
NeikiDev wants to merge 1 commit into
tsoding:mainfrom
NeikiDev:fix-242-read-entire-file

Conversation

@NeikiDev

Copy link
Copy Markdown

Fixes #242

nob_read_entire_file gets the file size via fseek/ftell and then reads exactly that many bytes. Files on virtual file systems like procfs report size 0, so the function "succeeds" but the string builder stays empty:

#define NOB_IMPLEMENTATION
#include "nob.h"

int main(void)
{
    Nob_String_Builder sb = {0};
    if (!nob_read_entire_file("/proc/version", &sb)) return 1;
    printf("read %zu bytes\n", sb.count);
    return 0;
}
$ ./main
read 0 bytes

Pipes and FIFOs are worse, fseek fails on them so you cannot read them at all:

[ERROR] Could not read file pipe: Illegal seek

The patch keeps the fseek/ftell size but only as a hint to preallocate the string builder. The actual reading is a fread loop until EOF, so the real content ends up in the builder no matter what size was reported. If the file is not seekable the preallocation is simply skipped. On failure sb->count is restored, so a failed read does not leave partial content in the builder, which matches the old behavior of not touching count on errors.

After the patch:

$ ./main
read 168 bytes

I also added tests/read_entire_file.c which covers regular files, appending to a non empty builder, a missing file, and reading from a FIFO (that last part is skipped on Windows).

Tested on Linux (gcc 14, both C and C++ mode) and macOS (clang), the whole test suite passes on both. One note: reading endless files like /dev/zero now keeps reading forever instead of returning an empty builder, but that is the same thing cat does.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

filesystem api fails for virtual file systems

1 participant