Skip to content

Commit

Permalink
Change calling conventions for filldir_t
Browse files Browse the repository at this point in the history
filldir_t instances (directory iterators callbacks) used to return 0 for
"OK, keep going" or -E... for "stop".  Note that it's *NOT* how the
error values are reported - the rules for those are callback-dependent
and ->iterate{,_shared}() instances only care about zero vs. non-zero
(look at emit_dir() and friends).

So let's just return bool ("should we keep going?") - it's less confusing
that way.  The choice between "true means keep going" and "true means
stop" is bikesheddable; we have two groups of callbacks -
	do something for everything in directory, until we run into problem
and
	find an entry in directory and do something to it.

The former tended to use 0/-E... conventions - -E<something> on failure.
The latter tended to use 0/1, 1 being "stop, we are done".
The callers treated anything non-zero as "stop", ignoring which
non-zero value did they get.

"true means stop" would be more natural for the second group; "true
means keep going" - for the first one.  I tried both variants and
the things like
	if allocation failed
		something = -ENOMEM;
		return true;
just looked unnatural and asking for trouble.

[folded suggestion from Matthew Wilcox <willy@infradead.org>]
Acked-by: Christian Brauner (Microsoft) <brauner@kernel.org>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
  • Loading branch information
Al Viro committed Aug 17, 2022
1 parent d6da19c commit 25885a3
Show file tree
Hide file tree
Showing 19 changed files with 153 additions and 155 deletions.
11 changes: 11 additions & 0 deletions Documentation/filesystems/porting.rst
Original file line number Diff line number Diff line change
Expand Up @@ -922,3 +922,14 @@ is provided - file_open_root_mnt(). In-tree users adjusted.
no_llseek is gone; don't set .llseek to that - just leave it NULL instead.
Checks for "does that file have llseek(2), or should it fail with ESPIPE"
should be done by looking at FMODE_LSEEK in file->f_mode.

---

*mandatory*

filldir_t (readdir callbacks) calling conventions have changed. Instead of
returning 0 or -E... it returns bool now. false means "no more" (as -E... used
to) and true - "keep going" (as 0 in old calling conventions). Rationale:
callers never looked at specific -E... values anyway. ->iterate() and
->iterate_shared() instance require no changes at all, all filldir_t ones in
the tree converted.
10 changes: 5 additions & 5 deletions arch/alpha/kernel/osf_sys.c
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ struct osf_dirent_callback {
int error;
};

static int
static bool
osf_filldir(struct dir_context *ctx, const char *name, int namlen,
loff_t offset, u64 ino, unsigned int d_type)
{
Expand All @@ -120,11 +120,11 @@ osf_filldir(struct dir_context *ctx, const char *name, int namlen,

buf->error = -EINVAL; /* only used if we fail */
if (reclen > buf->count)
return -EINVAL;
return false;
d_ino = ino;
if (sizeof(d_ino) < sizeof(ino) && d_ino != ino) {
buf->error = -EOVERFLOW;
return -EOVERFLOW;
return false;
}
if (buf->basep) {
if (put_user(offset, buf->basep))
Expand All @@ -141,10 +141,10 @@ osf_filldir(struct dir_context *ctx, const char *name, int namlen,
dirent = (void __user *)dirent + reclen;
buf->dirent = dirent;
buf->count -= reclen;
return 0;
return true;
Efault:
buf->error = -EFAULT;
return -EFAULT;
return false;
}

SYSCALL_DEFINE4(osf_getdirentries, unsigned int, fd,
Expand Down
23 changes: 10 additions & 13 deletions fs/afs/dir.c
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@ static int afs_readdir(struct file *file, struct dir_context *ctx);
static int afs_d_revalidate(struct dentry *dentry, unsigned int flags);
static int afs_d_delete(const struct dentry *dentry);
static void afs_d_iput(struct dentry *dentry, struct inode *inode);
static int afs_lookup_one_filldir(struct dir_context *ctx, const char *name, int nlen,
static bool afs_lookup_one_filldir(struct dir_context *ctx, const char *name, int nlen,
loff_t fpos, u64 ino, unsigned dtype);
static int afs_lookup_filldir(struct dir_context *ctx, const char *name, int nlen,
static bool afs_lookup_filldir(struct dir_context *ctx, const char *name, int nlen,
loff_t fpos, u64 ino, unsigned dtype);
static int afs_create(struct user_namespace *mnt_userns, struct inode *dir,
struct dentry *dentry, umode_t mode, bool excl);
Expand Down Expand Up @@ -568,7 +568,7 @@ static int afs_readdir(struct file *file, struct dir_context *ctx)
* - if afs_dir_iterate_block() spots this function, it'll pass the FID
* uniquifier through dtype
*/
static int afs_lookup_one_filldir(struct dir_context *ctx, const char *name,
static bool afs_lookup_one_filldir(struct dir_context *ctx, const char *name,
int nlen, loff_t fpos, u64 ino, unsigned dtype)
{
struct afs_lookup_one_cookie *cookie =
Expand All @@ -584,16 +584,16 @@ static int afs_lookup_one_filldir(struct dir_context *ctx, const char *name,

if (cookie->name.len != nlen ||
memcmp(cookie->name.name, name, nlen) != 0) {
_leave(" = 0 [no]");
return 0;
_leave(" = true [keep looking]");
return true;
}

cookie->fid.vnode = ino;
cookie->fid.unique = dtype;
cookie->found = 1;

_leave(" = -1 [found]");
return -1;
_leave(" = false [found]");
return false;
}

/*
Expand Down Expand Up @@ -636,12 +636,11 @@ static int afs_do_lookup_one(struct inode *dir, struct dentry *dentry,
* - if afs_dir_iterate_block() spots this function, it'll pass the FID
* uniquifier through dtype
*/
static int afs_lookup_filldir(struct dir_context *ctx, const char *name,
static bool afs_lookup_filldir(struct dir_context *ctx, const char *name,
int nlen, loff_t fpos, u64 ino, unsigned dtype)
{
struct afs_lookup_cookie *cookie =
container_of(ctx, struct afs_lookup_cookie, ctx);
int ret;

_enter("{%s,%u},%s,%u,,%llu,%u",
cookie->name.name, cookie->name.len, name, nlen,
Expand All @@ -663,12 +662,10 @@ static int afs_lookup_filldir(struct dir_context *ctx, const char *name,
cookie->fids[1].unique = dtype;
cookie->found = 1;
if (cookie->one_only)
return -1;
return false;
}

ret = cookie->nr_fids >= 50 ? -1 : 0;
_leave(" = %d", ret);
return ret;
return cookie->nr_fids < 50;
}

/*
Expand Down
38 changes: 16 additions & 22 deletions fs/ecryptfs/file.c
Original file line number Diff line number Diff line change
Expand Up @@ -53,26 +53,27 @@ struct ecryptfs_getdents_callback {
};

/* Inspired by generic filldir in fs/readdir.c */
static int
static bool
ecryptfs_filldir(struct dir_context *ctx, const char *lower_name,
int lower_namelen, loff_t offset, u64 ino, unsigned int d_type)
{
struct ecryptfs_getdents_callback *buf =
container_of(ctx, struct ecryptfs_getdents_callback, ctx);
size_t name_size;
char *name;
int rc;
int err;
bool res;

buf->filldir_called++;
rc = ecryptfs_decode_and_decrypt_filename(&name, &name_size,
buf->sb, lower_name,
lower_namelen);
if (rc) {
if (rc != -EINVAL) {
err = ecryptfs_decode_and_decrypt_filename(&name, &name_size,
buf->sb, lower_name,
lower_namelen);
if (err) {
if (err != -EINVAL) {
ecryptfs_printk(KERN_DEBUG,
"%s: Error attempting to decode and decrypt filename [%s]; rc = [%d]\n",
__func__, lower_name, rc);
return rc;
__func__, lower_name, err);
return false;
}

/* Mask -EINVAL errors as these are most likely due a plaintext
Expand All @@ -81,16 +82,15 @@ ecryptfs_filldir(struct dir_context *ctx, const char *lower_name,
* the "lost+found" dentry in the root directory of an Ext4
* filesystem.
*/
return 0;
return true;
}

buf->caller->pos = buf->ctx.pos;
rc = !dir_emit(buf->caller, name, name_size, ino, d_type);
res = dir_emit(buf->caller, name, name_size, ino, d_type);
kfree(name);
if (!rc)
if (res)
buf->entries_written++;

return rc;
return res;
}

/**
Expand All @@ -111,14 +111,8 @@ static int ecryptfs_readdir(struct file *file, struct dir_context *ctx)
lower_file = ecryptfs_file_to_lower(file);
rc = iterate_dir(lower_file, &buf.ctx);
ctx->pos = buf.ctx.pos;
if (rc < 0)
goto out;
if (buf.filldir_called && !buf.entries_written)
goto out;
if (rc >= 0)
fsstack_copy_attr_atime(inode,
file_inode(lower_file));
out:
if (rc >= 0 && (buf.entries_written || !buf.filldir_called))
fsstack_copy_attr_atime(inode, file_inode(lower_file));
return rc;
}

Expand Down
7 changes: 3 additions & 4 deletions fs/exportfs/expfs.c
Original file line number Diff line number Diff line change
Expand Up @@ -248,21 +248,20 @@ struct getdents_callback {
* A rather strange filldir function to capture
* the name matching the specified inode number.
*/
static int filldir_one(struct dir_context *ctx, const char *name, int len,
static bool filldir_one(struct dir_context *ctx, const char *name, int len,
loff_t pos, u64 ino, unsigned int d_type)
{
struct getdents_callback *buf =
container_of(ctx, struct getdents_callback, ctx);
int result = 0;

buf->sequence++;
if (buf->ino == ino && len <= NAME_MAX) {
memcpy(buf->name, name, len);
buf->name[len] = '\0';
buf->found = 1;
result = -1;
return false; // no more
}
return result;
return true;
}

/**
Expand Down
8 changes: 4 additions & 4 deletions fs/fat/dir.c
Original file line number Diff line number Diff line change
Expand Up @@ -705,7 +705,7 @@ static int fat_readdir(struct file *file, struct dir_context *ctx)
}

#define FAT_IOCTL_FILLDIR_FUNC(func, dirent_type) \
static int func(struct dir_context *ctx, const char *name, int name_len, \
static bool func(struct dir_context *ctx, const char *name, int name_len, \
loff_t offset, u64 ino, unsigned int d_type) \
{ \
struct fat_ioctl_filldir_callback *buf = \
Expand All @@ -714,7 +714,7 @@ static int func(struct dir_context *ctx, const char *name, int name_len, \
struct dirent_type __user *d2 = d1 + 1; \
\
if (buf->result) \
return -EINVAL; \
return false; \
buf->result++; \
\
if (name != NULL) { \
Expand Down Expand Up @@ -750,10 +750,10 @@ static int func(struct dir_context *ctx, const char *name, int name_len, \
put_user(short_len, &d1->d_reclen)) \
goto efault; \
} \
return 0; \
return true; \
efault: \
buf->result = -EFAULT; \
return -EFAULT; \
return false; \
}

FAT_IOCTL_FILLDIR_FUNC(fat_ioctl_filldir, __fat_dirent)
Expand Down
6 changes: 3 additions & 3 deletions fs/gfs2/export.c
Original file line number Diff line number Diff line change
Expand Up @@ -66,20 +66,20 @@ struct get_name_filldir {
char *name;
};

static int get_name_filldir(struct dir_context *ctx, const char *name,
static bool get_name_filldir(struct dir_context *ctx, const char *name,
int length, loff_t offset, u64 inum,
unsigned int type)
{
struct get_name_filldir *gnfd =
container_of(ctx, struct get_name_filldir, ctx);

if (inum != gnfd->inum.no_addr)
return 0;
return true;

memcpy(gnfd->name, name, length);
gnfd->name[length] = 0;

return 1;
return false;
}

static int gfs2_get_name(struct dentry *parent, char *name,
Expand Down
16 changes: 7 additions & 9 deletions fs/ksmbd/smb2pdu.c
Original file line number Diff line number Diff line change
Expand Up @@ -3779,7 +3779,7 @@ static int reserve_populate_dentry(struct ksmbd_dir_info *d_info,
return 0;
}

static int __query_dir(struct dir_context *ctx, const char *name, int namlen,
static bool __query_dir(struct dir_context *ctx, const char *name, int namlen,
loff_t offset, u64 ino, unsigned int d_type)
{
struct ksmbd_readdir_data *buf;
Expand All @@ -3793,22 +3793,20 @@ static int __query_dir(struct dir_context *ctx, const char *name, int namlen,

/* dot and dotdot entries are already reserved */
if (!strcmp(".", name) || !strcmp("..", name))
return 0;
return true;
if (ksmbd_share_veto_filename(priv->work->tcon->share_conf, name))
return 0;
return true;
if (!match_pattern(name, namlen, priv->search_pattern))
return 0;
return true;

d_info->name = name;
d_info->name_len = namlen;
rc = reserve_populate_dentry(d_info, priv->info_level);
if (rc)
return rc;
if (d_info->flags & SMB2_RETURN_SINGLE_ENTRY) {
return false;
if (d_info->flags & SMB2_RETURN_SINGLE_ENTRY)
d_info->out_buf_len = 0;
return 0;
}
return 0;
return true;
}

static void restart_ctx(struct dir_context *ctx)
Expand Down
14 changes: 6 additions & 8 deletions fs/ksmbd/vfs.c
Original file line number Diff line number Diff line change
Expand Up @@ -1105,17 +1105,15 @@ int ksmbd_vfs_unlink(struct user_namespace *user_ns,
return err;
}

static int __dir_empty(struct dir_context *ctx, const char *name, int namlen,
static bool __dir_empty(struct dir_context *ctx, const char *name, int namlen,
loff_t offset, u64 ino, unsigned int d_type)
{
struct ksmbd_readdir_data *buf;

buf = container_of(ctx, struct ksmbd_readdir_data, ctx);
buf->dirent_count++;

if (buf->dirent_count > 2)
return -ENOTEMPTY;
return 0;
return buf->dirent_count <= 2;
}

/**
Expand All @@ -1142,7 +1140,7 @@ int ksmbd_vfs_empty_dir(struct ksmbd_file *fp)
return err;
}

static int __caseless_lookup(struct dir_context *ctx, const char *name,
static bool __caseless_lookup(struct dir_context *ctx, const char *name,
int namlen, loff_t offset, u64 ino,
unsigned int d_type)
{
Expand All @@ -1151,13 +1149,13 @@ static int __caseless_lookup(struct dir_context *ctx, const char *name,
buf = container_of(ctx, struct ksmbd_readdir_data, ctx);

if (buf->used != namlen)
return 0;
return true;
if (!strncasecmp((char *)buf->private, name, namlen)) {
memcpy((char *)buf->private, name, namlen);
buf->dirent_count = 1;
return -EEXIST;
return false;
}
return 0;
return true;
}

/**
Expand Down
8 changes: 4 additions & 4 deletions fs/nfsd/nfs4recover.c
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ struct nfs4_dir_ctx {
struct list_head names;
};

static int
static bool
nfsd4_build_namelist(struct dir_context *__ctx, const char *name, int namlen,
loff_t offset, u64 ino, unsigned int d_type)
{
Expand All @@ -275,14 +275,14 @@ nfsd4_build_namelist(struct dir_context *__ctx, const char *name, int namlen,
struct name_list *entry;

if (namlen != HEXDIR_LEN - 1)
return 0;
return true;
entry = kmalloc(sizeof(struct name_list), GFP_KERNEL);
if (entry == NULL)
return -ENOMEM;
return false;
memcpy(entry->name, name, HEXDIR_LEN - 1);
entry->name[HEXDIR_LEN - 1] = '\0';
list_add(&entry->list, &ctx->names);
return 0;
return true;
}

static int
Expand Down
Loading

0 comments on commit 25885a3

Please sign in to comment.