zargony / fuse-rs

Rust library for filesystems in userspace (FUSE)
MIT License
1.05k stars 131 forks source link

Correct offset handling in readdir example #124

Closed wwylele closed 5 years ago

wwylele commented 5 years ago

I am using libfuse (fuse_lowleve_ops) documentation as the reference, assuming both rust-fuse and libfuse are thin layers on top of the data communication without altering values like +1 or -1.

The doc of fuse_add_direntry (assuming equivalent to ReplyDirectory::add) states that off (offset) is the offset of the next entry, and it does not need to be the actual physical position (read: an element index of a continuous array). This value is passed to the next readdir call as the offset parameter. According to the doc of readdir, "readdir() should skip over entries coming before the position defined by the off_t value." (read: the entry defined at offset should be returned in this call). The most intuitive design, when the entry forms a continuous array, is to use the offset as the index of the first entry that is going to be read in {the next call} (for reply.add) / {this call} (for readdir). This code change is to adopt this intuitive design.

The old code is not completely wrong. It almost works, but in a different way, which exploits the fact that "offset does not need to be the actual physical position". Here are two tables showing how exactly the old and the new approach work

The old code:

last index of element written to reply.add last offset written to reply.add offset passed to the next readdir index of the first element to write in the next readdir
0 0 0 0 (bug)
1 1 1 2
2 2 2 3
3 3 3 4

The new code:

last index of element written to reply.add last offset written to reply.add offset passed to the next readdir index of the first element to write in the next readdir
0 1 1 1
1 2 2 2
2 3 3 3
3 4 4 4

As shown in the table, the old code would give wrong result if the first readdir only read one entry at 0. This bug wasn't caught probably because callers rarely only read one entry for one call.

zargony commented 5 years ago

Wow, thanks a lot for the detailed description and analysis. This sounds very plausible to me.

/ping @illicitonion maybe you want to have a look too since you updated this part of the code in #105?

illicitonion commented 5 years ago

This seems pretty reasonable, and definitely has more research and justification put into it than I've done myself.