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

Fix returning d_type by readdir(3) in off == 0 mode #591

Merged
merged 2 commits into from
Mar 18, 2021
Merged

Fix returning d_type by readdir(3) in off == 0 mode #591

merged 2 commits into from
Mar 18, 2021

Conversation

jpandre
Copy link
Contributor

@jpandre jpandre commented Mar 15, 2021

When using the off == 0 mode and not using the readdir_plus mode,
the d_type was not returned, and the use_ino flag was not used for
returning d_ino.

Until a recent patch, this was also the case when using the readdir_plus
mode. It is likely the original developer intentionally used the
offs == 0 situation to trigger something while processing the first
call.

So this patch fixes the returned values for d_ino and d_type in both
with and without the readdir_plus mode. It also restores the original
design for the first returned entry in readdir_plus mode.

The test for the returned d_ino when off == 0 has been adjusted to
take the d_type into consideration.

When using the off == 0 mode and not using the readdir_plus mode,
the d_type was not returned, and the use_ino flag was not used for
returning d_ino.

Until a recent patch, this was also the case when using the readdir_plus
mode. It is likely the original developer intentionally used the
offs == 0 situation to trigger something while processing the first
call.

So this patch fixes the returned values for d_ino and d_type in both
with and without the readdir_plus mode. It also restores the original
design for the first returned entry in readdir_plus mode.

The test for the returned d_ino when off == 0 has been adjusted to
take the d_type into consideration.
lib/fuse.c Outdated
@@ -3566,7 +3566,7 @@ static int fill_dir_plus(void *dh_, const char *name, const struct stat *statp,
return 1;
}

if (statp && (flags & FUSE_FILL_DIR_PLUS)) {
if (off && statp && (flags & FUSE_FILL_DIR_PLUS)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure I understand this change. Why do we want to throw away most of the statp information when seeking is not supported?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I did not want to deprive any project of the stat information when available. This intent was not mentioned in the rationale which led to removing the "off == 0" check.

Rather, I think this check was there for a reason (possibly because some file system needed it), and I just wanted to avoid a possible regression.

Now, if the stat information is a useful feature on the first returned entry in readdir_plus mode (usually this is "."), this is a good rationale for removing the check, so let me keep it removed. This has no effect in my use cases.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got you. Yes, I'd rather leave this as-is.

Copy link
Contributor Author

@jpandre jpandre Mar 18, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This proposal keeps the off == 0 test as is and only deals with the non-plus readdir(3). Also the passthrough check has been extended to test readdir(3) in both modes.

The title should be changed consequently (which I probably cannot do).

When not using the readdir_plus mode, the d_type was not returned,
and the use_ino flag was not used for returning d_ino.

This patch fixes the returned values for d_ino and d_type by readdir(3)

The test for the returned value of d_ino has been adjusted to also
take the d_type into consideration and to check the returned values in
both basic readdir and readdir_plus modes. This is done by executing
the passthrough test twice.
@Nikratio
Copy link
Contributor

Thanks for working on this!

@Nikratio Nikratio merged commit bdd2d41 into libfuse:master Mar 18, 2021
@nh2
Copy link

nh2 commented Mar 30, 2021

I can confirm that this PR fixes readdir() always returning DT_UNKNOWN in sshfs, thus making many recursive directory traversal operations in many programming languages much faster, as they don't need to fall back to stat().

Thank you!

brauner pushed a commit to brauner/lxcfs that referenced this pull request Mar 13, 2022
Link: libfuse/libfuse#591
Signed-off-by: Christian Brauner (Microsoft) <christian.brauner@ubuntu.com>
brauner pushed a commit to brauner/lxcfs that referenced this pull request Mar 13, 2022
Link: libfuse/libfuse#591
Signed-off-by: Christian Brauner (Microsoft) <christian.brauner@ubuntu.com>
brauner pushed a commit to brauner/lxcfs that referenced this pull request Mar 13, 2022
Link: libfuse/libfuse#591
Signed-off-by: Christian Brauner (Microsoft) <christian.brauner@ubuntu.com>
brauner pushed a commit to brauner/lxcfs that referenced this pull request Mar 13, 2022
Link: libfuse/libfuse#591
Signed-off-by: Christian Brauner (Microsoft) <christian.brauner@ubuntu.com>
brauner pushed a commit to brauner/lxcfs that referenced this pull request Mar 13, 2022
Link: libfuse/libfuse#591
Signed-off-by: Christian Brauner (Microsoft) <christian.brauner@ubuntu.com>
brauner pushed a commit to brauner/lxcfs that referenced this pull request Mar 13, 2022
Link: libfuse/libfuse#591
Signed-off-by: Christian Brauner (Microsoft) <christian.brauner@ubuntu.com>
stgraber pushed a commit to lxc/lxcfs that referenced this pull request Mar 13, 2022
Link: libfuse/libfuse#591
Signed-off-by: Christian Brauner (Microsoft) <christian.brauner@ubuntu.com>
Foxboron pushed a commit to lxc-stable/lxcfs that referenced this pull request Mar 27, 2022
Link: libfuse/libfuse#591
Signed-off-by: Christian Brauner (Microsoft) <christian.brauner@ubuntu.com>
(cherry picked from commit 006db26)
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

Successfully merging this pull request may close these issues.

None yet

3 participants