Re: [PATCH] newlib: fix fseek optimization with SEEK_CUR

[Bastien Bouclet <bastien.bouclet@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1218 bytes --]

Hi Corinna,

Thank you for your answer.

> I checked this against upstream BSD versions.  OpenBSD and NetBSD
> operate like our code, including the flush, while FreeBSD uses its
> internal ftello and never flushed since the repository import back in
> 1994.

One difference I've noticed is that fflush does not invalidate the
stream read buffer in the BSD versions of libc.  In newlib this was
introduced in commit a8ef755c2776b8da4ea386360c1df74ce268c165.  Which
is probably why OpenBSD and NetBSD can call fflush in fseek with
SEEK_CUR.

> Can we be sure this works as desired on append streams as well?

Regarding the append streams, it's worth noting there is another call
to fflush at the beginning of fseek in that case.  I've written a small
test program to verify they did not regress in simple cases.

> Also, given that this is changing very basic code, nobody is unaffected.

I would like to see the performance issue fixed one way or another.
The systems I target do not have a page cache, the extra reads have a
noticeable impact on user experience.  Another other option could be
having a compile time option for disabling the code in fflush that
forces a disk access on the next read.

Regards,
Bastien

[-- Attachment #2: stdio-append.c --]
[-- Type: text/x-csrc, Size: 1559 bytes --]

#include <errno.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

int main() {
	FILE *fp = fopen("test.txt", "a+");
	if (fp == NULL) {
		puts("failed to open test file in append mode");
		exit(1);
	}

	size_t written = fwrite("1234567890", 10, 1, fp);
	if (written != 1) {
		puts("failed to write");
		exit(1);
	}

	int ret = fseek(fp, -8, SEEK_CUR);
	if (ret == -1) {
		puts("failed to seek");
		exit(1);
	}

	char read_buf[100];
	memset(read_buf, 0, sizeof(read_buf));

	size_t read = fread(read_buf, 2, 1, fp);
	if (read != 1) {
		printf("failed to read %d\n", errno);
		exit(1);
	}

	if (strcmp(read_buf, "34") != 0) {
		printf("unexpected read value %s\n", read_buf);
		exit(1);
	}

	ret = ungetc('a', fp);
	if (ret != 'a') {
		puts("failed to ungetc");
		exit(1);
	}

	ret = fseek(fp, 3, SEEK_CUR);
	if (ret == -1) {
		puts("failed to seek");
		exit(1);
	}

	read = fread(read_buf, 2, 1, fp);
	if (read != 1) {
		printf("failed to read %d\n", errno);
		exit(1);
	}

	if (strcmp(read_buf, "78") != 0) {
		printf("unexpected read value %s\n", read_buf);
		exit(1);
	}

	written = fwrite("0987654321", 10, 1, fp);
	if (written != 1) {
		puts("failed to write");
		exit(1);
	}

	ret = fseek(fp, -20, SEEK_CUR);
	if (ret == -1) {
		puts("failed to seek");
		exit(1);
	}

	read = fread(read_buf, 20, 1, fp);
	if (read != 1) {
		printf("failed to read %d\n", errno);
		exit(1);
	}

	if (strcmp(read_buf, "12345678900987654321") != 0) {
		printf("unexpected read value %s\n", read_buf);
		exit(1);
	}

	fclose(fp);

	puts("success");

	return 0;
}