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
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions ChangeLog.rst
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
* Fix returning d_ino and d_type from readdir() in offset==0 mode.

libfuse 3.10.2 (2021-02-05)
===========================

Expand Down
2 changes: 1 addition & 1 deletion example/passthrough.c
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ static int xmp_readdir(const char *path, void *buf, fuse_fill_dir_t filler,
memset(&st, 0, sizeof(st));
st.st_ino = de->d_ino;
st.st_mode = de->d_type << 12;
if (filler(buf, de->d_name, &st, 0, FUSE_FILL_DIR_PLUS))
if (filler(buf, de->d_name, &st, 0, 0))
break;
}

Expand Down
7 changes: 6 additions & 1 deletion lib/fuse.c
Original file line number Diff line number Diff line change
Expand Up @@ -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).

e.attr = *statp;

if (!is_dot_or_dotdot(name)) {
Expand All @@ -3578,6 +3578,11 @@ static int fill_dir_plus(void *dh_, const char *name, const struct stat *statp,
}
} else {
e.attr.st_ino = FUSE_UNKNOWN_INO;
if (statp) {
e.attr.st_mode = statp->st_mode;
if (f->conf.use_ino)
e.attr.st_ino = statp->st_ino;
}
if (!f->conf.use_ino && f->conf.readdir_ino) {
e.attr.st_ino = (ino_t)
lookup_nodeid(f, dh->nodeid, name);
Expand Down
18 changes: 15 additions & 3 deletions test/readdir_inode.c
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
/*
* Prints each directory entry and its inode as returned by 'readdir'.
* Prints each directory entry, its inode and d_type as returned by 'readdir'.
* Skips '.' and '..' because readdir is not required to return them and
* some of our examples don't.
* some of our examples don't. However if they are returned, their d_type
* should be valid.
*/

#include <stdio.h>
Expand Down Expand Up @@ -30,7 +31,18 @@ int main(int argc, char* argv[])
dent = readdir(dirp);
while (dent != NULL) {
if (strcmp(dent->d_name, ".") != 0 && strcmp(dent->d_name, "..") != 0) {
printf("%llu %s\n", (unsigned long long)dent->d_ino, dent->d_name);
printf("%llu %d %s\n", (unsigned long long)dent->d_ino,
(int)dent->d_type, dent->d_name);
if ((long long)dent->d_ino <= 1)
fprintf(stderr,"%s : bad d_ino %lld\n",
(unsigned long long)dent->d_ino);
if ((dent->d_type < 1) || (dent->d_type > 15))
fprintf(stderr,"%s : bad d_type %d\n",
(int)dent->d_type);
} else {
if (dent->d_type != DT_DIR)
fprintf(stderr,"%s : bad d_type %d\n",
(int)dent->d_type);
}
dent = readdir(dirp);
}
Expand Down