mikaku / Fiwix

A UNIX-like kernel for the i386 architecture
https://www.fiwix.org
Other
401 stars 32 forks source link

Read from start of file right after opening with O_APPEND #76

Closed rick-masters closed 6 months ago

rick-masters commented 6 months ago

In order to be consistent with Linux behavior and expectations of some software, the file position for reads should be set to zero when a file is opened with O_APPEND.

This may seem odd, but O_APPEND only specifies that writes will start at the end of the file. The position of reads, however, is not specified in the POSIX standard and is left up to the implementation. Linux for whatever reason chose to leave the read position at zero. This is not well documented, but you can find anectdotes confirming this, for example: https://cygwin.cygwin.narkive.com/6x8daLJe/bug-fopen-a-does-not-seek-to-end-of-file-until-some-write-operation https://github.com/JuliaLang/julia/issues/3374#issuecomment-19404458

This can be verified empirically with the following program:

#include <stdlib.h>
#include <stdio.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <unistd.h>
#include <fcntl.h>

int main() {
        FILE* testfile = fopen("testfile", "w");
        fprintf(testfile, "hello\n");
        fclose(testfile);

/* Two different variations are provided here */

#if 1
        int testfd = open("testfile", O_RDWR|O_APPEND);
        int pos = lseek(testfd, 0, SEEK_CUR);
        printf("position: %d\n", pos);
        close(testfd);
#else
        testfile = fopen("testfile", "a+");
        int pos = ftell(testfile);
        printf("position: %d\n", pos);
        fclose(testfile);
#endif
}

On Linux, this will output:

position: 0

On Fiwix, this will output:

position: 6

Certain version of the m4 macro processor which is part of autotools depends on the read position being at the start of the file. Version 1.4.10 of m4 opens "diversion" temporary files with "a+" append mode when re-inserting diverted content. (See m4_tmpopen called by insert_diversion_helper.) It then reads the file and assumes it will be from the beginning.

I think m4 changed this in later versions, but to avoid any problems like this in the future, it would be preferable if Fiwix worked the same was as Linux.

mikaku commented 6 months ago

This is so weird behavior... Have you tested that after an open() with O_APPEND writes aren't affected by this change? I mean, writes start from the end of the file?

rick-masters commented 6 months ago

Yes, it still writes to the end because your code still sets the position to the end of the file on every write. See lines 95-97: https://github.com/mikaku/Fiwix/blob/43a3a949a6ee201f768010a5b4a3aa8fd536da8a/fs/ext2/file.c#L82-L97

If you add this line before the close() call, the "world" line is appended to the end of testfile:

        write(testfd, "world\n", 6);
mikaku commented 6 months ago

I've tested this change with a complete package building and it has not affected the results. Thank you very much.