Skip to content
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

File not truncated to actual size after calling fs_close #24462

Closed
arvinf opened this issue Apr 17, 2020 · 7 comments
Closed

File not truncated to actual size after calling fs_close #24462

arvinf opened this issue Apr 17, 2020 · 7 comments
Assignees

Comments

@arvinf
Copy link
Contributor

arvinf commented Apr 17, 2020

Describe the bug
When a file is rewritten with less data and then closed the file is not truncated to the most recent data. The old data is still in the file.

To Reproduce

  1. Create a file and write 100 bytes of arbitrary data.
memset(buffer, 0xAA, 100);
fs_open(&file, '/foo/bar.txt');
fs_write(&file, buffer, 100);
fs_close(&file);
  1. Re-open the file and write 50 bytes of different data.
memset(buffer, 0x55, 50);
fs_open(&file, '/foo/bar.txt');
fs_write(&file, buffer, 50);
fs_close(&file);
  1. Re-open the file, check file size and read contents.
memset(buffer, 0xBB, 100);
fs_open(&file, '/foo/bar.txt');
fs_seek(&file, 0, FS_SEEK_END);
size = fs_tell(&file); 
fs_seek(&file, 0, FS_SEEK_SET);
fs_read(&file, buffer, size);
fs_close(&file);

// size = 100
// buffer = 50 bytes of 0x55 followed by 50 bytes of 0xAA

Expected behavior
When a file is closed, it should be truncated to the current data. Old data should be discarded.

Impact
I can currently work around this by calling fs_truncate after calling fs_close but this is unusual, unexpected behaviour and not consistent with POSIX file operations.

Environment

  • Zephyr OS build v2.1.99-ncs1-1141-g616c7d5becd4
  • File ops on a flash partition mounted with littlefs
@arvinf arvinf added the bug The issue is a bug, or the PR is fixing a bug label Apr 17, 2020
@pabigot pabigot self-assigned this Apr 18, 2020
@pabigot
Copy link
Collaborator

pabigot commented Apr 18, 2020

The file system is behaving as it should. Zephyr's file system (and particularly littlefs) follows POSIX file system standards: if you open an existing file, you are positioned at the start, but the existing file content is not removed. So what your example does is:

  • Create a file with 100 x 0xAA and close it;
  • Reopen it and overwrite the first 50 octets with 0x55, then close it;
  • Seek confirms file length is 100
  • Read confirms file content is 50 0x55 followed by 50 0xAA

As an aside this command:

    memset(buffer, 0xBB, 0x00);

does not change the buffer content (the length to set is zero). Since you don't check the return code from the read you aren't confirming that the file content is correct: a potential (but not present) failure mode would be that the read failed and the buffer content was left unchanged which would be 50 0x55 followed by 50 0xAA.

If you add this to step 2 just before the close:

    fs_trunc(&file, 50);

you will discard the remainder of the file. Or you could fs_trunc(&file, 0) right after the open and also get the behavior you were expecting.

@pabigot pabigot closed this as completed Apr 18, 2020
@pabigot pabigot added question and removed bug The issue is a bug, or the PR is fixing a bug labels Apr 18, 2020
@pabigot
Copy link
Collaborator

pabigot commented Apr 18, 2020

Reading your original comment more completely: Yes, this is compatible with POSIX, given that Zephyr doesn't allow you to provide flags. The open it's simulating acts as though it was given O_CREATE | O_RDRW but not O_TRUNC. So if you want to truncate you have to do it after you open.

@arvinf
Copy link
Contributor Author

arvinf commented Apr 20, 2020

Thank you for your answer. I was expecting the behaviour to be more inline with POSIX fopen, which always truncates on write.

Just a suggestion, that maybe this should be made explicit in the documentation. Most programmers typically deal with fopen[1], rather than open when manipulating the file system and would naturally expect fs_open to behave like fopen.

Also, the code that I provided was from memory, to provide a schematic. The memset line was a typo. Thank you for pointing it out. It has has been corrected.

  1. POSIX-2017

@pabigot
Copy link
Collaborator

pabigot commented Apr 20, 2020

I see that with POSIX fopen(path, "w") (or "w+") it would truncate the existing file. But Zephyr's fs_open isn't given any mode flags and it allows both read and write, so the POSIX equivalent mode is "r+". I don't think generally people will expect opening an existing file to cause its contents to be wiped out.

I don't think there's anything to be done here. But thank you for providing a clear example of what you were doing; it did make verifying that littlefs is behaving correctly easier.

@arvinf
Copy link
Contributor Author

arvinf commented Apr 20, 2020

That is true but without flags it is effectively "rw" since you can read and write.

Thank you for answering my question and resolving the issue.

@pabigot
Copy link
Collaborator

pabigot commented Apr 20, 2020

But "rw" is not a defined configuration for the mode parameter to POSIX or ISO-C fopen. Which honestly surprised me, since I remember using that back in the day. Learning every day....

And you're welcome.

@arvinf
Copy link
Contributor Author

arvinf commented Apr 20, 2020

TIL!

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

No branches or pull requests

2 participants