Truncate through the descriptor when there is one - #66
Open
Ebrathul wants to merge 2 commits into
Open
Conversation
openVFSfuse_truncate() ignored fuse_file_info and always truncated by path, so the file's mode was re-checked on every call. Truncating through a descriptor that is already open for writing then failed with EACCES once the file had been made read-only, where POSIX requires it to succeed -- the same shape as the write() problem. libfuse passes fi when the truncation comes from an open descriptor and null when it comes from a path-based truncate(), so the null case keeps the previous behaviour. Assisted-by: Claude Opus 5 (Anthropic)
Use the printf length modifier matching the intmax_t conversion and preserve truncate errors across debug logging. Assisted-by: Codex:GPT-5
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Found while running openvfs against my own account with my own tool and the desktop-client
plug-in from nextcloud/desktop#10635 and working through what a sync root has to
survive before it can hold ordinary files, read-only ones included.
openVFSfuse_truncate()ignoresfuse_file_infoand always truncates by path:So the file's mode is re-checked on every call. Truncating through a descriptor
already open for writing fails with
EACCESonce the file has been maderead-only, where POSIX requires it to succeed — the permission check belongs to
open(), not to each operation on the descriptor it returned.libfuse passes
fiwhen the truncation arrives through an open descriptor andnull when it comes from a path-based
truncate(), so the two cases are alreadydistinguishable; the information was simply unused.
Reproduction
Write through a descriptor, make the file read-only, then truncate through the
same descriptor. Same script on tmpfs for comparison:
The change
Use the descriptor when it is there:
The log uses an
intmax_tconversion foroff_t, and the failure path saveserrnobefore debug logging so formatting or syslog cannot change the errorreturned to FUSE.
The null case is the previous behaviour untouched, so a plain
truncate()on apath — including on a dehydrated placeholder — goes exactly where it went before.
Verified both ways:
ftruncate()through a descriptor now succeeds on aread-only file, and
truncate -s 3on a path still shortens the file to threebytes.
I did not change what happens when a dehydrated placeholder is truncated by
path. Truncating a file to zero arguably should not hydrate it first, but that is
issue #38's territory.
No test, for the same reason as the sibling pull request:
mainhas noharness that mounts a filesystem, and #64 is already introducing one for the
socket layer. The script above is the whole reproduction.
Relationship to the other pull requests
Same shape of defect as the
write()half of my other pull request — anoperation given a descriptor and using the path instead — but an independent fix
for an independent symptom, so it is here on its own rather than as a third
commit there. Either can go in without the other.
Independent of #61–#64, none of which touch
openVFSfuse_truncate(). Appliescleanly to
mainatcbdeeef, and I also cherry-picked it onto #64's head andbuilt there —
ctestgreen including #64's newsocketthreadtest, andftruncate()through a descriptor on a read-only file verified again on thatcombination. Tested, not assumed.
Environment
Linux 7.1.8 (CachyOS), libfuse 3.18.2, Btrfs.
Applied to a fresh clone of
mainatcbdeeefand built there. BuiltRelWithDebInfowith gcc 16.2.1, gcc 15 and clang 22 — clean on all three,ctestgreen, clang-format clean. Theftruncate(fi->fh, size)line warns under-Wconversionexactly as the existingpread(fi->fh, ...)inread()does, andno other way.