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.

Sign up for GitHub

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

sys/fs/constfs: drop dummy implementations #17651

Merged
merged 2 commits into from
May 19, 2022
Merged

sys/fs/constfs: drop dummy implementations #17651

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
991f8e8
sys/fs/constfs: drop dummy implementations
benpicco Feb 14, 2022
a587069
vfs: return -EROFS if remove operation is not implemented
benpicco May 18, 2022
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
61 changes: 0 additions & 61 deletions sys/fs/constfs/constfs.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,46 +33,34 @@
#include "debug.h"

/* File system operations */
static int constfs_mount(vfs_mount_t *mountp);
static int constfs_umount(vfs_mount_t *mountp);
static int constfs_unlink(vfs_mount_t *mountp, const char *name);
static int constfs_stat(vfs_mount_t *mountp, const char *restrict name, struct stat *restrict buf);
static int constfs_statvfs(vfs_mount_t *mountp, const char *restrict path, struct statvfs *restrict buf);

/* File operations */
static int constfs_close(vfs_file_t *filp);
static int constfs_fstat(vfs_file_t *filp, struct stat *buf);
static off_t constfs_lseek(vfs_file_t *filp, off_t off, int whence);
static int constfs_open(vfs_file_t *filp, const char *name, int flags, mode_t mode, const char *abs_path);
static ssize_t constfs_read(vfs_file_t *filp, void *dest, size_t nbytes);
static ssize_t constfs_write(vfs_file_t *filp, const void *src, size_t nbytes);

/* Directory operations */
static int constfs_opendir(vfs_DIR *dirp, const char *dirname, const char *abs_path);
static int constfs_readdir(vfs_DIR *dirp, vfs_dirent_t *entry);
static int constfs_closedir(vfs_DIR *dirp);

static const vfs_file_system_ops_t constfs_fs_ops = {
.mount = constfs_mount,
.umount = constfs_umount,
.unlink = constfs_unlink,
.statvfs = constfs_statvfs,
.stat = constfs_stat,
};

static const vfs_file_ops_t constfs_file_ops = {
.close = constfs_close,
.fstat = constfs_fstat,
.lseek = constfs_lseek,
.open = constfs_open,
.read = constfs_read,
.write = constfs_write,
};

static const vfs_dir_ops_t constfs_dir_ops = {
.opendir = constfs_opendir,
.readdir = constfs_readdir,
.closedir = constfs_closedir,
};

const vfs_file_system_t constfs_file_system = {
Expand All @@ -91,28 +79,6 @@ const vfs_file_system_t constfs_file_system = {
*/
static void _constfs_write_stat(const constfs_file_t *fp, struct stat *restrict buf);

static int constfs_mount(vfs_mount_t *mountp)
{
/* perform any extra initialization here */
(void) mountp; /* prevent warning: unused parameter */
return 0;
}

static int constfs_umount(vfs_mount_t *mountp)
{
/* free resources and perform any clean up here */
(void) mountp; /* prevent warning: unused parameter */
return 0;
}

static int constfs_unlink(vfs_mount_t *mountp, const char *name)
{
/* Removing files is prohibited */
(void) mountp; /* prevent warning: unused parameter */
(void) name; /* prevent warning: unused parameter */
return -EROFS;
Copy link
Member

Choose a reason for hiding this comment

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

This is technically a change from -EPERM (which is the default) to -EROFS; probably OK (but may be worth a second thought).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Shouldn't the default then rather be changed to -EROFS? The function not being implemented is not a permission issue.

Copy link
Member

Choose a reason for hiding this comment

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

I have no strong opinions on the concrete standard error values we return due to POSIXish conventions; EROFS sounds just as good as EPERM to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I decided to change it to -EROFS in VFS.

}

static int constfs_stat(vfs_mount_t *mountp, const char *restrict name, struct stat *restrict buf)
{
(void) name;
Expand Down Expand Up @@ -162,13 +128,6 @@ static int constfs_statvfs(vfs_mount_t *mountp, const char *restrict path, struc
return 0;
}

static int constfs_close(vfs_file_t *filp)
{
/* perform any necessary clean ups */
(void) filp; /* prevent warning: unused parameter */
return 0;
}

static int constfs_fstat(vfs_file_t *filp, struct stat *buf)
{
constfs_file_t *fp = filp->private_data.ptr;
Expand Down Expand Up @@ -244,18 +203,6 @@ static ssize_t constfs_read(vfs_file_t *filp, void *dest, size_t nbytes)
return nbytes;
}

static ssize_t constfs_write(vfs_file_t *filp, const void *src, size_t nbytes)
{
DEBUG("constfs_write: %p, %p, %lu\n", (void *)filp, src, (unsigned long)nbytes);
/* Read only file system */
DEBUG("constfs_write: read only FS\n");
/* prevent warning: unused parameter */
(void) filp;
(void) src;
(void) nbytes;
return -EBADF;
Copy link
Member

Choose a reason for hiding this comment

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

This is technically a change (vfs_write without a write returns -EINVAL), but I'd say it's for the better.

}

static int constfs_opendir(vfs_DIR *dirp, const char *dirname, const char *abs_path)
{
(void) abs_path;
Expand Down Expand Up @@ -298,14 +245,6 @@ static int constfs_readdir(vfs_DIR *dirp, vfs_dirent_t *entry)
return 1;
}

static int constfs_closedir(vfs_DIR *dirp)
{
/* Just an example, it's not necessary to define closedir if there is
* nothing to clean up */
(void) dirp;
return 0;
}

static void _constfs_write_stat(const constfs_file_t *fp, struct stat *restrict buf)
{
/* buffer is cleared by vfs already */
Expand Down
8 changes: 4 additions & 4 deletions sys/vfs/vfs.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -583,7 +583,7 @@ int vfs_rename(const char *from_path, const char *to_path)
DEBUG("vfs_rename: rename not supported by fs!\n");
/* remember to decrement the open_files count */
atomic_fetch_sub(&mountp->open_files, 1);
return -EPERM;
return -EROFS;
}
const char *rel_to;
vfs_mount_t *mountp_to;
Expand Down Expand Up @@ -642,7 +642,7 @@ int vfs_unlink(const char *name)
DEBUG("vfs_unlink: unlink not supported by fs!\n");
/* remember to decrement the open_files count */
atomic_fetch_sub(&mountp->open_files, 1);
return -EPERM;
return -EROFS;
}
res = mountp->fs->fs_op->unlink(mountp, rel_path);
DEBUG("vfs_unlink: unlink %p, \"%s\"", (void *)mountp, rel_path);
Expand Down Expand Up @@ -679,7 +679,7 @@ int vfs_mkdir(const char *name, mode_t mode)
DEBUG("vfs_mkdir: mkdir not supported by fs!\n");
/* remember to decrement the open_files count */
atomic_fetch_sub(&mountp->open_files, 1);
return -EPERM;
return -EROFS;
}
res = mountp->fs->fs_op->mkdir(mountp, rel_path, mode);
DEBUG("vfs_mkdir: mkdir %p, \"%s\"", (void *)mountp, rel_path);
Expand Down Expand Up @@ -716,7 +716,7 @@ int vfs_rmdir(const char *name)
DEBUG("vfs_rmdir: rmdir not supported by fs!\n");
/* remember to decrement the open_files count */
atomic_fetch_sub(&mountp->open_files, 1);
return -EPERM;
return -EROFS;
}
res = mountp->fs->fs_op->rmdir(mountp, rel_path);
DEBUG("vfs_rmdir: rmdir %p, \"%s\"", (void *)mountp, rel_path);
Expand Down
8 changes: 4 additions & 4 deletions tests/unittests/tests-vfs/tests-vfs-file-system-ops.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,28 +96,28 @@ static void test_vfs_null_fs_ops_rename(void)
{
TEST_ASSERT_EQUAL_INT(0, _test_vfs_fs_op_mount_res);
int res = vfs_rename("/test/foo", "/test/bar");
TEST_ASSERT_EQUAL_INT(-EPERM, res);
TEST_ASSERT_EQUAL_INT(-EROFS, res);
}

static void test_vfs_null_fs_ops_unlink(void)
{
TEST_ASSERT_EQUAL_INT(0, _test_vfs_fs_op_mount_res);
int res = vfs_unlink("/test/foo");
TEST_ASSERT_EQUAL_INT(-EPERM, res);
TEST_ASSERT_EQUAL_INT(-EROFS, res);
}

static void test_vfs_null_fs_ops_mkdir(void)
{
TEST_ASSERT_EQUAL_INT(0, _test_vfs_fs_op_mount_res);
int res = vfs_mkdir("/test/foodir", 0);
TEST_ASSERT_EQUAL_INT(-EPERM, res);
TEST_ASSERT_EQUAL_INT(-EROFS, res);
}

static void test_vfs_null_fs_ops_rmdir(void)
{
TEST_ASSERT_EQUAL_INT(0, _test_vfs_fs_op_mount_res);
int res = vfs_rmdir("/test/foodir");
TEST_ASSERT_EQUAL_INT(-EPERM, res);
TEST_ASSERT_EQUAL_INT(-EROFS, res);
}

static void test_vfs_null_fs_ops_stat(void)
Expand Down