* [PATCH v5 1/3] ubifs: prefer kstrtobool_from_user() over custom helper
@ 2026-04-15 7:18 Dmitry Antipov
2026-04-15 7:18 ` [PATCH v5 2/3] ubifs: use strscpy() and kmemdup_nul() where appropriate Dmitry Antipov
2026-04-15 7:18 ` [PATCH v5 3/3] ubifs: avoid redundant calls to memset() Dmitry Antipov
0 siblings, 2 replies; 7+ messages in thread
From: Dmitry Antipov @ 2026-04-15 7:18 UTC (permalink / raw)
To: Zhihao Cheng, Richard Weinberger; +Cc: linux-mtd, Dmitry Antipov
Adjust 'dfs_file_write()' and 'dfs_global_file_write()' to prefer generic
'kstrtobool_from_user()' over an ad-hoc 'interpret_user_input()' helper,
thus making the latter not needed anymore.
Reviewed-by: Zhihao Cheng <chengzhihao1@huawei.com>
Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
---
v3 and upwards: bump version to match the series
v2: add Reviewed-by: and bump version to match the series
---
fs/ubifs/debug.c | 44 ++++++++++----------------------------------
1 file changed, 10 insertions(+), 34 deletions(-)
diff --git a/fs/ubifs/debug.c b/fs/ubifs/debug.c
index 5794de5a9069..9e0468dc72d3 100644
--- a/fs/ubifs/debug.c
+++ b/fs/ubifs/debug.c
@@ -2732,39 +2732,14 @@ static ssize_t dfs_file_read(struct file *file, char __user *u, size_t count,
return provide_user_output(val, u, count, ppos);
}
-/**
- * interpret_user_input - interpret user debugfs file input.
- * @u: user-provided buffer with the input
- * @count: buffer size
- *
- * This is a helper function which interpret user input to a boolean UBIFS
- * debugfs file. Returns %0 or %1 in case of success and a negative error code
- * in case of failure.
- */
-static int interpret_user_input(const char __user *u, size_t count)
-{
- size_t buf_size;
- char buf[8];
-
- buf_size = min_t(size_t, count, (sizeof(buf) - 1));
- if (copy_from_user(buf, u, buf_size))
- return -EFAULT;
-
- if (buf[0] == '1')
- return 1;
- else if (buf[0] == '0')
- return 0;
-
- return -EINVAL;
-}
-
static ssize_t dfs_file_write(struct file *file, const char __user *u,
size_t count, loff_t *ppos)
{
struct ubifs_info *c = file->private_data;
struct ubifs_debug_info *d = c->dbg;
struct dentry *dent = file->f_path.dentry;
- int val;
+ bool val;
+ int ret;
if (file->f_path.dentry == d->dfs_dump_lprops) {
ubifs_dump_lprops(c);
@@ -2781,9 +2756,9 @@ static ssize_t dfs_file_write(struct file *file, const char __user *u,
return count;
}
- val = interpret_user_input(u, count);
- if (val < 0)
- return val;
+ ret = kstrtobool_from_user(u, count, &val);
+ if (unlikely(ret))
+ return ret;
if (dent == d->dfs_chk_gen)
d->chk_gen = val;
@@ -2926,11 +2901,12 @@ static ssize_t dfs_global_file_write(struct file *file, const char __user *u,
size_t count, loff_t *ppos)
{
struct dentry *dent = file->f_path.dentry;
- int val;
+ bool val;
+ int ret;
- val = interpret_user_input(u, count);
- if (val < 0)
- return val;
+ ret = kstrtobool_from_user(u, count, &val);
+ if (unlikely(ret))
+ return ret;
if (dent == dfs_chk_gen)
ubifs_dbg.chk_gen = val;
--
2.53.0
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v5 2/3] ubifs: use strscpy() and kmemdup_nul() where appropriate
2026-04-15 7:18 [PATCH v5 1/3] ubifs: prefer kstrtobool_from_user() over custom helper Dmitry Antipov
@ 2026-04-15 7:18 ` Dmitry Antipov
2026-04-16 3:37 ` Zhihao Cheng
2026-04-15 7:18 ` [PATCH v5 3/3] ubifs: avoid redundant calls to memset() Dmitry Antipov
1 sibling, 1 reply; 7+ messages in thread
From: Dmitry Antipov @ 2026-04-15 7:18 UTC (permalink / raw)
To: Zhihao Cheng, Richard Weinberger; +Cc: linux-mtd, Dmitry Antipov
Go closer to the modern kernel API and use 'strscpy()' and 'kmemdup_nul()'
over an ad-hoc ensure-to-have-'\0' quirks where appropriate.
Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
---
v5: switch to fname_len() + 1 to specify strscpy() target buffer size
v4: once again to not forget a filesystem-native (little) to CPU endian swap
v3: fix strscpy() usage as noticed by Zhihao
v2: initial version to join the series
---
fs/ubifs/journal.c | 18 ++++++------------
fs/ubifs/replay.c | 3 +--
fs/ubifs/super.c | 8 ++------
3 files changed, 9 insertions(+), 20 deletions(-)
diff --git a/fs/ubifs/journal.c b/fs/ubifs/journal.c
index 40a95a2fad50..8d7f0843d353 100644
--- a/fs/ubifs/journal.c
+++ b/fs/ubifs/journal.c
@@ -729,8 +729,7 @@ int ubifs_jnl_update(struct ubifs_info *c, const struct inode *dir,
dent->inum = deletion ? 0 : cpu_to_le64(inode->i_ino);
dent->type = get_dent_type(inode->i_mode);
dent->nlen = cpu_to_le16(fname_len(nm));
- memcpy(dent->name, fname_name(nm), fname_len(nm));
- dent->name[fname_len(nm)] = '\0';
+ strscpy(dent->name, fname_name(nm), fname_len(nm) + 1);
set_dent_cookie(c, dent);
zero_dent_node_unused(dent);
@@ -1232,8 +1231,7 @@ int ubifs_jnl_xrename(struct ubifs_info *c, const struct inode *fst_dir,
dent1->inum = cpu_to_le64(fst_inode->i_ino);
dent1->type = get_dent_type(fst_inode->i_mode);
dent1->nlen = cpu_to_le16(fname_len(snd_nm));
- memcpy(dent1->name, fname_name(snd_nm), fname_len(snd_nm));
- dent1->name[fname_len(snd_nm)] = '\0';
+ strscpy(dent1->name, fname_name(snd_nm), fname_len(snd_nm) + 1);
set_dent_cookie(c, dent1);
zero_dent_node_unused(dent1);
ubifs_prep_grp_node(c, dent1, dlen1, 0);
@@ -1248,8 +1246,7 @@ int ubifs_jnl_xrename(struct ubifs_info *c, const struct inode *fst_dir,
dent2->inum = cpu_to_le64(snd_inode->i_ino);
dent2->type = get_dent_type(snd_inode->i_mode);
dent2->nlen = cpu_to_le16(fname_len(fst_nm));
- memcpy(dent2->name, fname_name(fst_nm), fname_len(fst_nm));
- dent2->name[fname_len(fst_nm)] = '\0';
+ strscpy(dent2->name, fname_name(fst_nm), fname_len(fst_nm) + 1);
set_dent_cookie(c, dent2);
zero_dent_node_unused(dent2);
ubifs_prep_grp_node(c, dent2, dlen2, 0);
@@ -1424,8 +1421,7 @@ int ubifs_jnl_rename(struct ubifs_info *c, const struct inode *old_dir,
dent->inum = cpu_to_le64(old_inode->i_ino);
dent->type = get_dent_type(old_inode->i_mode);
dent->nlen = cpu_to_le16(fname_len(new_nm));
- memcpy(dent->name, fname_name(new_nm), fname_len(new_nm));
- dent->name[fname_len(new_nm)] = '\0';
+ strscpy(dent->name, fname_name(new_nm), fname_len(new_nm) + 1);
set_dent_cookie(c, dent);
zero_dent_node_unused(dent);
ubifs_prep_grp_node(c, dent, dlen1, 0);
@@ -1446,8 +1442,7 @@ int ubifs_jnl_rename(struct ubifs_info *c, const struct inode *old_dir,
dent2->type = DT_UNKNOWN;
}
dent2->nlen = cpu_to_le16(fname_len(old_nm));
- memcpy(dent2->name, fname_name(old_nm), fname_len(old_nm));
- dent2->name[fname_len(old_nm)] = '\0';
+ strscpy(dent2->name, fname_name(old_nm), fname_len(old_nm) + 1);
set_dent_cookie(c, dent2);
zero_dent_node_unused(dent2);
ubifs_prep_grp_node(c, dent2, dlen2, 0);
@@ -1897,8 +1892,7 @@ int ubifs_jnl_delete_xattr(struct ubifs_info *c, const struct inode *host,
xent->inum = 0;
xent->type = get_dent_type(inode->i_mode);
xent->nlen = cpu_to_le16(fname_len(nm));
- memcpy(xent->name, fname_name(nm), fname_len(nm));
- xent->name[fname_len(nm)] = '\0';
+ strscpy(xent->name, fname_name(nm), fname_len(nm) + 1);
zero_dent_node_unused(xent);
ubifs_prep_grp_node(c, xent, xlen, 0);
diff --git a/fs/ubifs/replay.c b/fs/ubifs/replay.c
index a9a568f4a868..ef6ae63792d1 100644
--- a/fs/ubifs/replay.c
+++ b/fs/ubifs/replay.c
@@ -463,8 +463,7 @@ static int insert_dent(struct ubifs_info *c, int lnum, int offs, int len,
r->sqnum = sqnum;
key_copy(c, key, &r->key);
fname_len(&r->nm) = nlen;
- memcpy(nbuf, name, nlen);
- nbuf[nlen] = '\0';
+ strscpy(nbuf, name, nlen + 1);
fname_name(&r->nm) = nbuf;
list_add_tail(&r->list, &c->replay_list);
diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
index 9a77d8b64ffa..048c21271db1 100644
--- a/fs/ubifs/super.c
+++ b/fs/ubifs/super.c
@@ -168,13 +168,11 @@ struct inode *ubifs_iget(struct super_block *sb, unsigned long inum)
inode->i_op = &ubifs_file_inode_operations;
inode->i_fop = &ubifs_file_operations;
if (ui->xattr) {
- ui->data = kmalloc(ui->data_len + 1, GFP_NOFS);
+ ui->data = kmemdup_nul(ino->data, ui->data_len, GFP_NOFS);
if (!ui->data) {
err = -ENOMEM;
goto out_ino;
}
- memcpy(ui->data, ino->data, ui->data_len);
- ((char *)ui->data)[ui->data_len] = '\0';
} else if (ui->data_len != 0) {
err = 10;
goto out_invalid;
@@ -194,13 +192,11 @@ struct inode *ubifs_iget(struct super_block *sb, unsigned long inum)
err = 12;
goto out_invalid;
}
- ui->data = kmalloc(ui->data_len + 1, GFP_NOFS);
+ ui->data = kmemdup_nul(ino->data, ui->data_len, GFP_NOFS);
if (!ui->data) {
err = -ENOMEM;
goto out_ino;
}
- memcpy(ui->data, ino->data, ui->data_len);
- ((char *)ui->data)[ui->data_len] = '\0';
break;
case S_IFBLK:
case S_IFCHR:
--
2.53.0
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v5 3/3] ubifs: avoid redundant calls to memset()
2026-04-15 7:18 [PATCH v5 1/3] ubifs: prefer kstrtobool_from_user() over custom helper Dmitry Antipov
2026-04-15 7:18 ` [PATCH v5 2/3] ubifs: use strscpy() and kmemdup_nul() where appropriate Dmitry Antipov
@ 2026-04-15 7:18 ` Dmitry Antipov
2026-04-16 3:52 ` Zhihao Cheng
1 sibling, 1 reply; 7+ messages in thread
From: Dmitry Antipov @ 2026-04-15 7:18 UTC (permalink / raw)
To: Zhihao Cheng, Richard Weinberger; +Cc: linux-mtd, Dmitry Antipov
Rely on partial implicit initialization of 'struct ubifs_budget_req'
objects and so simplify 'ubifs_release_dirty_inode_budget()', 'do_rename()'
and 'do_truncation()' by dropping explicit calls to 'memset()'.
Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
---
v5: initial version to join the series
---
fs/ubifs/budget.c | 9 ++++-----
fs/ubifs/dir.c | 4 +---
fs/ubifs/file.c | 10 +++++-----
3 files changed, 10 insertions(+), 13 deletions(-)
diff --git a/fs/ubifs/budget.c b/fs/ubifs/budget.c
index d76eb7b39f56..a73f1e969f7c 100644
--- a/fs/ubifs/budget.c
+++ b/fs/ubifs/budget.c
@@ -590,11 +590,10 @@ void ubifs_convert_page_budget(struct ubifs_info *c)
void ubifs_release_dirty_inode_budget(struct ubifs_info *c,
struct ubifs_inode *ui)
{
- struct ubifs_budget_req req;
-
- memset(&req, 0, sizeof(struct ubifs_budget_req));
- /* The "no space" flags will be cleared because dd_growth is > 0 */
- req.dd_growth = c->bi.inode_budget + ALIGN(ui->data_len, 8);
+ struct ubifs_budget_req req = {
+ /* The "no space" flags will be cleared because dd_growth is > 0 */
+ .dd_growth = c->bi.inode_budget + ALIGN(ui->data_len, 8)
+ };
ubifs_release_budget(c, &req);
}
diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
index 86d41e077e4d..c1602e8aff01 100644
--- a/fs/ubifs/dir.c
+++ b/fs/ubifs/dir.c
@@ -1335,7 +1335,7 @@ static int do_rename(struct inode *old_dir, struct dentry *old_dentry,
.dirtied_ino = 3 };
struct ubifs_budget_req ino_req = { .dirtied_ino = 1,
.dirtied_ino_d = ALIGN(old_inode_ui->data_len, 8) };
- struct ubifs_budget_req wht_req;
+ struct ubifs_budget_req wht_req = { .new_ino = 1 };
unsigned int saved_nlink;
struct fscrypt_name old_nm, new_nm;
@@ -1422,8 +1422,6 @@ static int do_rename(struct inode *old_dir, struct dentry *old_dentry,
whiteout_ui->data_len = ubifs_encode_dev(dev, MKDEV(0, 0));
ubifs_assert(c, !whiteout_ui->dirty);
- memset(&wht_req, 0, sizeof(struct ubifs_budget_req));
- wht_req.new_ino = 1;
wht_req.new_ino_d = ALIGN(whiteout_ui->data_len, 8);
/*
* To avoid deadlock between space budget (holds ui_mutex and
diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c
index e73c28b12f97..35061a587e3c 100644
--- a/fs/ubifs/file.c
+++ b/fs/ubifs/file.c
@@ -1101,13 +1101,16 @@ static int do_truncation(struct ubifs_info *c, struct inode *inode,
const struct iattr *attr)
{
int err;
- struct ubifs_budget_req req;
+ struct ubifs_budget_req req = {
+ .dirtied_ino = 1,
+ /* A funny way to budget for truncation node */
+ .dirtied_ino_d = UBIFS_TRUN_NODE_SZ
+ };
loff_t old_size = inode->i_size, new_size = attr->ia_size;
int offset = new_size & (UBIFS_BLOCK_SIZE - 1), budgeted = 1;
struct ubifs_inode *ui = ubifs_inode(inode);
dbg_gen("ino %llu, size %lld -> %lld", inode->i_ino, old_size, new_size);
- memset(&req, 0, sizeof(struct ubifs_budget_req));
/*
* If this is truncation to a smaller size, and we do not truncate on a
@@ -1117,9 +1120,6 @@ static int do_truncation(struct ubifs_info *c, struct inode *inode,
if (new_size & (UBIFS_BLOCK_SIZE - 1))
req.dirtied_page = 1;
- req.dirtied_ino = 1;
- /* A funny way to budget for truncation node */
- req.dirtied_ino_d = UBIFS_TRUN_NODE_SZ;
err = ubifs_budget_space(c, &req);
if (err) {
/*
--
2.53.0
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v5 2/3] ubifs: use strscpy() and kmemdup_nul() where appropriate
2026-04-15 7:18 ` [PATCH v5 2/3] ubifs: use strscpy() and kmemdup_nul() where appropriate Dmitry Antipov
@ 2026-04-16 3:37 ` Zhihao Cheng
2026-04-16 5:09 ` Dmitry Antipov
0 siblings, 1 reply; 7+ messages in thread
From: Zhihao Cheng @ 2026-04-16 3:37 UTC (permalink / raw)
To: Dmitry Antipov, Richard Weinberger; +Cc: linux-mtd
在 2026/4/15 15:18, Dmitry Antipov 写道:
> Go closer to the modern kernel API and use 'strscpy()' and 'kmemdup_nul()'
> over an ad-hoc ensure-to-have-'\0' quirks where appropriate.
>
> Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
> ---
> v5: switch to fname_len() + 1 to specify strscpy() target buffer size
> v4: once again to not forget a filesystem-native (little) to CPU endian swap
> v3: fix strscpy() usage as noticed by Zhihao
> v2: initial version to join the series
> ---
> fs/ubifs/journal.c | 18 ++++++------------
> fs/ubifs/replay.c | 3 +--
> fs/ubifs/super.c | 8 ++------
> 3 files changed, 9 insertions(+), 20 deletions(-)
>
> diff --git a/fs/ubifs/journal.c b/fs/ubifs/journal.c
> index 40a95a2fad50..8d7f0843d353 100644
> --- a/fs/ubifs/journal.c
> +++ b/fs/ubifs/journal.c
> @@ -729,8 +729,7 @@ int ubifs_jnl_update(struct ubifs_info *c, const struct inode *dir,
> dent->inum = deletion ? 0 : cpu_to_le64(inode->i_ino);
> dent->type = get_dent_type(inode->i_mode);
> dent->nlen = cpu_to_le16(fname_len(nm));
> - memcpy(dent->name, fname_name(nm), fname_len(nm));
> - dent->name[fname_len(nm)] = '\0';
> + strscpy(dent->name, fname_name(nm), fname_len(nm) + 1);
Hi Dmitry,
I notice that some architectures can accelerate the implementation of
memcpy(), and the length of dentry name could be 255, I suggest to keep
the orignal implementation.
> set_dent_cookie(c, dent);
>
> zero_dent_node_unused(dent);
> @@ -1232,8 +1231,7 @@ int ubifs_jnl_xrename(struct ubifs_info *c, const struct inode *fst_dir,
> dent1->inum = cpu_to_le64(fst_inode->i_ino);
> dent1->type = get_dent_type(fst_inode->i_mode);
> dent1->nlen = cpu_to_le16(fname_len(snd_nm));
> - memcpy(dent1->name, fname_name(snd_nm), fname_len(snd_nm));
> - dent1->name[fname_len(snd_nm)] = '\0';
> + strscpy(dent1->name, fname_name(snd_nm), fname_len(snd_nm) + 1);
> set_dent_cookie(c, dent1);
> zero_dent_node_unused(dent1);
> ubifs_prep_grp_node(c, dent1, dlen1, 0);
> @@ -1248,8 +1246,7 @@ int ubifs_jnl_xrename(struct ubifs_info *c, const struct inode *fst_dir,
> dent2->inum = cpu_to_le64(snd_inode->i_ino);
> dent2->type = get_dent_type(snd_inode->i_mode);
> dent2->nlen = cpu_to_le16(fname_len(fst_nm));
> - memcpy(dent2->name, fname_name(fst_nm), fname_len(fst_nm));
> - dent2->name[fname_len(fst_nm)] = '\0';
> + strscpy(dent2->name, fname_name(fst_nm), fname_len(fst_nm) + 1);
> set_dent_cookie(c, dent2);
> zero_dent_node_unused(dent2);
> ubifs_prep_grp_node(c, dent2, dlen2, 0);
> @@ -1424,8 +1421,7 @@ int ubifs_jnl_rename(struct ubifs_info *c, const struct inode *old_dir,
> dent->inum = cpu_to_le64(old_inode->i_ino);
> dent->type = get_dent_type(old_inode->i_mode);
> dent->nlen = cpu_to_le16(fname_len(new_nm));
> - memcpy(dent->name, fname_name(new_nm), fname_len(new_nm));
> - dent->name[fname_len(new_nm)] = '\0';
> + strscpy(dent->name, fname_name(new_nm), fname_len(new_nm) + 1);
> set_dent_cookie(c, dent);
> zero_dent_node_unused(dent);
> ubifs_prep_grp_node(c, dent, dlen1, 0);
> @@ -1446,8 +1442,7 @@ int ubifs_jnl_rename(struct ubifs_info *c, const struct inode *old_dir,
> dent2->type = DT_UNKNOWN;
> }
> dent2->nlen = cpu_to_le16(fname_len(old_nm));
> - memcpy(dent2->name, fname_name(old_nm), fname_len(old_nm));
> - dent2->name[fname_len(old_nm)] = '\0';
> + strscpy(dent2->name, fname_name(old_nm), fname_len(old_nm) + 1);
> set_dent_cookie(c, dent2);
> zero_dent_node_unused(dent2);
> ubifs_prep_grp_node(c, dent2, dlen2, 0);
> @@ -1897,8 +1892,7 @@ int ubifs_jnl_delete_xattr(struct ubifs_info *c, const struct inode *host,
> xent->inum = 0;
> xent->type = get_dent_type(inode->i_mode);
> xent->nlen = cpu_to_le16(fname_len(nm));
> - memcpy(xent->name, fname_name(nm), fname_len(nm));
> - xent->name[fname_len(nm)] = '\0';
> + strscpy(xent->name, fname_name(nm), fname_len(nm) + 1);
> zero_dent_node_unused(xent);
> ubifs_prep_grp_node(c, xent, xlen, 0);
>
> diff --git a/fs/ubifs/replay.c b/fs/ubifs/replay.c
> index a9a568f4a868..ef6ae63792d1 100644
> --- a/fs/ubifs/replay.c
> +++ b/fs/ubifs/replay.c
> @@ -463,8 +463,7 @@ static int insert_dent(struct ubifs_info *c, int lnum, int offs, int len,
> r->sqnum = sqnum;
> key_copy(c, key, &r->key);
> fname_len(&r->nm) = nlen;
> - memcpy(nbuf, name, nlen);
> - nbuf[nlen] = '\0';
> + strscpy(nbuf, name, nlen + 1);
> fname_name(&r->nm) = nbuf;
>
> list_add_tail(&r->list, &c->replay_list);
> diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
> index 9a77d8b64ffa..048c21271db1 100644
> --- a/fs/ubifs/super.c
> +++ b/fs/ubifs/super.c
> @@ -168,13 +168,11 @@ struct inode *ubifs_iget(struct super_block *sb, unsigned long inum)
> inode->i_op = &ubifs_file_inode_operations;
> inode->i_fop = &ubifs_file_operations;
> if (ui->xattr) {
> - ui->data = kmalloc(ui->data_len + 1, GFP_NOFS);
> + ui->data = kmemdup_nul(ino->data, ui->data_len, GFP_NOFS);
> if (!ui->data) {
> err = -ENOMEM;
> goto out_ino;
> }
> - memcpy(ui->data, ino->data, ui->data_len);
> - ((char *)ui->data)[ui->data_len] = '\0';
> } else if (ui->data_len != 0) {
> err = 10;
> goto out_invalid;
> @@ -194,13 +192,11 @@ struct inode *ubifs_iget(struct super_block *sb, unsigned long inum)
> err = 12;
> goto out_invalid;
> }
> - ui->data = kmalloc(ui->data_len + 1, GFP_NOFS);
> + ui->data = kmemdup_nul(ino->data, ui->data_len, GFP_NOFS);
> if (!ui->data) {
> err = -ENOMEM;
> goto out_ino;
> }
> - memcpy(ui->data, ino->data, ui->data_len);
> - ((char *)ui->data)[ui->data_len] = '\0';
> break;
> case S_IFBLK:
> case S_IFCHR:
>
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v5 3/3] ubifs: avoid redundant calls to memset()
2026-04-15 7:18 ` [PATCH v5 3/3] ubifs: avoid redundant calls to memset() Dmitry Antipov
@ 2026-04-16 3:52 ` Zhihao Cheng
0 siblings, 0 replies; 7+ messages in thread
From: Zhihao Cheng @ 2026-04-16 3:52 UTC (permalink / raw)
To: Dmitry Antipov, Richard Weinberger; +Cc: linux-mtd
在 2026/4/15 15:18, Dmitry Antipov 写道:
> Rely on partial implicit initialization of 'struct ubifs_budget_req'
> objects and so simplify 'ubifs_release_dirty_inode_budget()', 'do_rename()'
> and 'do_truncation()' by dropping explicit calls to 'memset()'.
>
> Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
> ---
> v5: initial version to join the series
> ---
> fs/ubifs/budget.c | 9 ++++-----
> fs/ubifs/dir.c | 4 +---
> fs/ubifs/file.c | 10 +++++-----
> 3 files changed, 10 insertions(+), 13 deletions(-)
>
> diff --git a/fs/ubifs/budget.c b/fs/ubifs/budget.c
> index d76eb7b39f56..a73f1e969f7c 100644
> --- a/fs/ubifs/budget.c
> +++ b/fs/ubifs/budget.c
> @@ -590,11 +590,10 @@ void ubifs_convert_page_budget(struct ubifs_info *c)
> void ubifs_release_dirty_inode_budget(struct ubifs_info *c,
> struct ubifs_inode *ui)
> {
> - struct ubifs_budget_req req;
> -
> - memset(&req, 0, sizeof(struct ubifs_budget_req));
> - /* The "no space" flags will be cleared because dd_growth is > 0 */
> - req.dd_growth = c->bi.inode_budget + ALIGN(ui->data_len, 8);
> + struct ubifs_budget_req req = {
> + /* The "no space" flags will be cleared because dd_growth is > 0 */
> + .dd_growth = c->bi.inode_budget + ALIGN(ui->data_len, 8)
> + };
> ubifs_release_budget(c, &req);
> }
>
> diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
> index 86d41e077e4d..c1602e8aff01 100644
> --- a/fs/ubifs/dir.c
> +++ b/fs/ubifs/dir.c
> @@ -1335,7 +1335,7 @@ static int do_rename(struct inode *old_dir, struct dentry *old_dentry,
> .dirtied_ino = 3 };
> struct ubifs_budget_req ino_req = { .dirtied_ino = 1,
> .dirtied_ino_d = ALIGN(old_inode_ui->data_len, 8) };
> - struct ubifs_budget_req wht_req;
> + struct ubifs_budget_req wht_req = { .new_ino = 1 };
> unsigned int saved_nlink;
> struct fscrypt_name old_nm, new_nm;
>
> @@ -1422,8 +1422,6 @@ static int do_rename(struct inode *old_dir, struct dentry *old_dentry,
> whiteout_ui->data_len = ubifs_encode_dev(dev, MKDEV(0, 0));
> ubifs_assert(c, !whiteout_ui->dirty);
>
> - memset(&wht_req, 0, sizeof(struct ubifs_budget_req));
> - wht_req.new_ino = 1;
> wht_req.new_ino_d = ALIGN(whiteout_ui->data_len, 8);
> /*
> * To avoid deadlock between space budget (holds ui_mutex and
I prefer to keep the orginal logic for do_rename(). it makes 'whiteout'
and 'wht_req' be initialized in the same code block, which is more readable.
> diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c
> index e73c28b12f97..35061a587e3c 100644
> --- a/fs/ubifs/file.c
> +++ b/fs/ubifs/file.c
> @@ -1101,13 +1101,16 @@ static int do_truncation(struct ubifs_info *c, struct inode *inode,
> const struct iattr *attr)
> {
> int err;
> - struct ubifs_budget_req req;
> + struct ubifs_budget_req req = {
> + .dirtied_ino = 1,
> + /* A funny way to budget for truncation node */
> + .dirtied_ino_d = UBIFS_TRUN_NODE_SZ
> + };
> loff_t old_size = inode->i_size, new_size = attr->ia_size;
> int offset = new_size & (UBIFS_BLOCK_SIZE - 1), budgeted = 1;
> struct ubifs_inode *ui = ubifs_inode(inode);
>
> dbg_gen("ino %llu, size %lld -> %lld", inode->i_ino, old_size, new_size);
> - memset(&req, 0, sizeof(struct ubifs_budget_req));
>
> /*
> * If this is truncation to a smaller size, and we do not truncate on a
> @@ -1117,9 +1120,6 @@ static int do_truncation(struct ubifs_info *c, struct inode *inode,
> if (new_size & (UBIFS_BLOCK_SIZE - 1))
> req.dirtied_page = 1;
>
> - req.dirtied_ino = 1;
> - /* A funny way to budget for truncation node */
> - req.dirtied_ino_d = UBIFS_TRUN_NODE_SZ;
> err = ubifs_budget_space(c, &req);
> if (err) {
> /*
>
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v5 2/3] ubifs: use strscpy() and kmemdup_nul() where appropriate
2026-04-16 3:37 ` Zhihao Cheng
@ 2026-04-16 5:09 ` Dmitry Antipov
2026-04-16 6:41 ` Zhihao Cheng
0 siblings, 1 reply; 7+ messages in thread
From: Dmitry Antipov @ 2026-04-16 5:09 UTC (permalink / raw)
To: Zhihao Cheng, Richard Weinberger; +Cc: linux-mtd
On Thu, 2026-04-16 at 11:37 +0800, Zhihao Cheng wrote:
> > - memcpy(dent->name, fname_name(nm), fname_len(nm));
> > - dent->name[fname_len(nm)] = '\0';
> > + strscpy(dent->name, fname_name(nm), fname_len(nm) + 1);
>
> Hi Dmitry,
> I notice that some architectures can accelerate the implementation of
> memcpy(), and the length of dentry name could be 255, I suggest to keep
> the orignal implementation.
What about
memcpy_and_pad(dent->name, fname_len(nm) + 1, fname_name(nm), fname_len(nm), 0)?
I have a strong suspicion that all of the '\0' things should be left to
library functions. So this is not a style but rather an overall design point -
if you're inserting '\0' at your own, most likely you're doing something
wrong, at least because you're duplicating something which is already
implemented in a library function.
Dmitry
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v5 2/3] ubifs: use strscpy() and kmemdup_nul() where appropriate
2026-04-16 5:09 ` Dmitry Antipov
@ 2026-04-16 6:41 ` Zhihao Cheng
0 siblings, 0 replies; 7+ messages in thread
From: Zhihao Cheng @ 2026-04-16 6:41 UTC (permalink / raw)
To: Dmitry Antipov, Richard Weinberger; +Cc: linux-mtd
在 2026/4/16 13:09, Dmitry Antipov 写道:
> On Thu, 2026-04-16 at 11:37 +0800, Zhihao Cheng wrote:
>
>>> - memcpy(dent->name, fname_name(nm), fname_len(nm));
>>> - dent->name[fname_len(nm)] = '\0';
>>> + strscpy(dent->name, fname_name(nm), fname_len(nm) + 1);
>>
>> Hi Dmitry,
>> I notice that some architectures can accelerate the implementation of
>> memcpy(), and the length of dentry name could be 255, I suggest to keep
>> the orignal implementation.
>
> What about
>
> memcpy_and_pad(dent->name, fname_len(nm) + 1, fname_name(nm), fname_len(nm), 0)?
I weakly accept memcpy_and_pad().
>
> I have a strong suspicion that all of the '\0' things should be left to
> library functions. So this is not a style but rather an overall design point -
> if you're inserting '\0' at your own, most likely you're doing something
> wrong, at least because you're duplicating something which is already
> implemented in a library function.
Well, kernel developers, of course, need to understand when and how to
set the trailing '\0'. There are many other places setting trailing
'\0'(eg. xfs_init_local_fork, xfs_symlink_remote_read,
ext4_xattr_move_to_block, etc.). I don't think there's a problem with
this; the developers know what they're doing.
>
> Dmitry
> .
>
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-04-16 6:42 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-15 7:18 [PATCH v5 1/3] ubifs: prefer kstrtobool_from_user() over custom helper Dmitry Antipov
2026-04-15 7:18 ` [PATCH v5 2/3] ubifs: use strscpy() and kmemdup_nul() where appropriate Dmitry Antipov
2026-04-16 3:37 ` Zhihao Cheng
2026-04-16 5:09 ` Dmitry Antipov
2026-04-16 6:41 ` Zhihao Cheng
2026-04-15 7:18 ` [PATCH v5 3/3] ubifs: avoid redundant calls to memset() Dmitry Antipov
2026-04-16 3:52 ` Zhihao Cheng
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox