| Message ID | 20240325125713.95035-1-wangzhaolong1@huawei.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [V2] ubifs: correct UBIFS_DFS_DIR_LEN macro definition and improve code clarity | expand |
在 2024/3/25 20:57, ZhaoLong Wang 写道: > The UBIFS_DFS_DIR_LEN macro, which defines the maximum length of the UBIFS > debugfs directory name, has an incorrect formula and misleading comments. > The current formula is (3 + 1 + 2*2 + 1), which assumes that both UBI device > number and volume ID are limited to 2 characters. However, UBI device number > ranges from 0 to 37 (2 characters), and volume ID ranges from 0 to 127 (up > to 3 characters). > > Although the current code works due to the cancellation of mathematical > errors (9 + 1 = 10, which matches the correct UBIFS_DFS_DIR_LEN value), it > can lead to confusion and potential issues in the future. > > This patch aims to improve the code clarity and maintainability by making > the following changes: > > 1. Corrects the UBIFS_DFS_DIR_LEN macro definition to (3 + 1 + 2 + 3 + 1), > accommodating the maximum lengths of both UBI device number and volume ID, > plus the separators and null terminator. > 2. Updates the snprintf calls to use UBIFS_DFS_DIR_LEN instead of > UBIFS_DFS_DIR_LEN + 1, removing the unnecessary +1. > 3. Modifies the error checks to compare against UBIFS_DFS_DIR_LEN using >= > instead of >, aligning with the corrected macro definition. > 4. Removes the redundant +1 in the dfs_dir_name array definitions in ubi.h > and debug.h. > 5. Renames the duplicated UBIFS_DFS_DIR_LEN and UBIFS_DFS_DIR_NAME macros in > the sysfs-related code (sysfs.c and ubifs.h) to UBIFS_SYSFS_DIR_LEN and > UBIFS_SYSFS_DIR_NAME, respectively, to differentiate their usage from the > debugfs constants. > [...] > diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h > index 1f3ea879d93a..549e147c5ce5 100644 > --- a/fs/ubifs/ubifs.h > +++ b/fs/ubifs/ubifs.h > @@ -159,10 +159,11 @@ > > /* > * The UBIFS sysfs directory name pattern and maximum name length (3 for "ubi" > - * + 1 for "_" and plus 2x2 for 2 UBI numbers and 1 for the trailing zero byte. > + * + 1 for "_" and 2 for UBI device numbers and 3 for volume number and 1 for > + * the trailing zero byte. > */ > -#define UBIFS_DFS_DIR_NAME "ubi%d_%d" > -#define UBIFS_DFS_DIR_LEN (3 + 1 + 2*2 + 1) > +#define UBIFS_SYSFS_DIR_NAME "ubi%d_%d" > +#define UBIFS_SYSFS_DIR_LEN (3 + 1 + 2 + 3 + 1) I don't thinks this renaming is necessary, just remove it from "ubifs.h" like v1 does. > > /* > * Lockdep classes for UBIFS inode @ui_mutex. >
diff --git a/drivers/mtd/ubi/debug.c b/drivers/mtd/ubi/debug.c index d57f52bd2ff3..9ec3b8b6a0aa 100644 --- a/drivers/mtd/ubi/debug.c +++ b/drivers/mtd/ubi/debug.c @@ -598,9 +598,9 @@ int ubi_debugfs_init_dev(struct ubi_device *ubi) if (!IS_ENABLED(CONFIG_DEBUG_FS)) return 0; - n = snprintf(d->dfs_dir_name, UBI_DFS_DIR_LEN + 1, UBI_DFS_DIR_NAME, + n = snprintf(d->dfs_dir_name, UBI_DFS_DIR_LEN, UBI_DFS_DIR_NAME, ubi->ubi_num); - if (n > UBI_DFS_DIR_LEN) { + if (n >= UBI_DFS_DIR_LEN) { /* The array size is too small */ return -EINVAL; } diff --git a/drivers/mtd/ubi/ubi.h b/drivers/mtd/ubi/ubi.h index 32009a24869e..da4e53ef5b0a 100644 --- a/drivers/mtd/ubi/ubi.h +++ b/drivers/mtd/ubi/ubi.h @@ -420,7 +420,7 @@ struct ubi_debug_info { unsigned int power_cut_min; unsigned int power_cut_max; unsigned int emulate_failures; - char dfs_dir_name[UBI_DFS_DIR_LEN + 1]; + char dfs_dir_name[UBI_DFS_DIR_LEN]; struct dentry *dfs_dir; struct dentry *dfs_chk_gen; struct dentry *dfs_chk_io; diff --git a/fs/ubifs/debug.c b/fs/ubifs/debug.c index ac77ac1fd73e..d91cec93d968 100644 --- a/fs/ubifs/debug.c +++ b/fs/ubifs/debug.c @@ -2827,9 +2827,9 @@ void dbg_debugfs_init_fs(struct ubifs_info *c) const char *fname; struct ubifs_debug_info *d = c->dbg; - n = snprintf(d->dfs_dir_name, UBIFS_DFS_DIR_LEN + 1, UBIFS_DFS_DIR_NAME, + n = snprintf(d->dfs_dir_name, UBIFS_DFS_DIR_LEN, UBIFS_DFS_DIR_NAME, c->vi.ubi_num, c->vi.vol_id); - if (n > UBIFS_DFS_DIR_LEN) { + if (n >= UBIFS_DFS_DIR_LEN) { /* The array size is too small */ return; } diff --git a/fs/ubifs/debug.h b/fs/ubifs/debug.h index ed966108da80..d425861e6b82 100644 --- a/fs/ubifs/debug.h +++ b/fs/ubifs/debug.h @@ -19,10 +19,11 @@ typedef int (*dbg_znode_callback)(struct ubifs_info *c, /* * The UBIFS debugfs directory name pattern and maximum name length (3 for "ubi" - * + 1 for "_" and plus 2x2 for 2 UBI numbers and 1 for the trailing zero byte. + * + 1 for "_" and 2 for UBI device numbers and 3 for volume number and 1 for + * the trailing zero byte. */ #define UBIFS_DFS_DIR_NAME "ubi%d_%d" -#define UBIFS_DFS_DIR_LEN (3 + 1 + 2*2 + 1) +#define UBIFS_DFS_DIR_LEN (3 + 1 + 2 + 3 + 1) /** * ubifs_debug_info - per-FS debugging information. @@ -103,7 +104,7 @@ struct ubifs_debug_info { unsigned int chk_fs:1; unsigned int tst_rcvry:1; - char dfs_dir_name[UBIFS_DFS_DIR_LEN + 1]; + char dfs_dir_name[UBIFS_DFS_DIR_LEN]; struct dentry *dfs_dir; struct dentry *dfs_dump_lprops; struct dentry *dfs_dump_budg; diff --git a/fs/ubifs/sysfs.c b/fs/ubifs/sysfs.c index 1c958148bb87..7b30aac42ac5 100644 --- a/fs/ubifs/sysfs.c +++ b/fs/ubifs/sysfs.c @@ -91,17 +91,17 @@ static struct kset ubifs_kset = { int ubifs_sysfs_register(struct ubifs_info *c) { int ret, n; - char dfs_dir_name[UBIFS_DFS_DIR_LEN+1]; + char dfs_dir_name[UBIFS_SYSFS_DIR_LEN]; c->stats = kzalloc(sizeof(struct ubifs_stats_info), GFP_KERNEL); if (!c->stats) { ret = -ENOMEM; goto out_last; } - n = snprintf(dfs_dir_name, UBIFS_DFS_DIR_LEN + 1, UBIFS_DFS_DIR_NAME, + n = snprintf(dfs_dir_name, UBIFS_SYSFS_DIR_LEN, UBIFS_SYSFS_DIR_NAME, c->vi.ubi_num, c->vi.vol_id); - if (n > UBIFS_DFS_DIR_LEN) { + if (n >= UBIFS_SYSFS_DIR_LEN) { /* The array size is too small */ ret = -EINVAL; goto out_free; diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h index 1f3ea879d93a..549e147c5ce5 100644 --- a/fs/ubifs/ubifs.h +++ b/fs/ubifs/ubifs.h @@ -159,10 +159,11 @@ /* * The UBIFS sysfs directory name pattern and maximum name length (3 for "ubi" - * + 1 for "_" and plus 2x2 for 2 UBI numbers and 1 for the trailing zero byte. + * + 1 for "_" and 2 for UBI device numbers and 3 for volume number and 1 for + * the trailing zero byte. */ -#define UBIFS_DFS_DIR_NAME "ubi%d_%d" -#define UBIFS_DFS_DIR_LEN (3 + 1 + 2*2 + 1) +#define UBIFS_SYSFS_DIR_NAME "ubi%d_%d" +#define UBIFS_SYSFS_DIR_LEN (3 + 1 + 2 + 3 + 1) /* * Lockdep classes for UBIFS inode @ui_mutex.
The UBIFS_DFS_DIR_LEN macro, which defines the maximum length of the UBIFS debugfs directory name, has an incorrect formula and misleading comments. The current formula is (3 + 1 + 2*2 + 1), which assumes that both UBI device number and volume ID are limited to 2 characters. However, UBI device number ranges from 0 to 37 (2 characters), and volume ID ranges from 0 to 127 (up to 3 characters). Although the current code works due to the cancellation of mathematical errors (9 + 1 = 10, which matches the correct UBIFS_DFS_DIR_LEN value), it can lead to confusion and potential issues in the future. This patch aims to improve the code clarity and maintainability by making the following changes: 1. Corrects the UBIFS_DFS_DIR_LEN macro definition to (3 + 1 + 2 + 3 + 1), accommodating the maximum lengths of both UBI device number and volume ID, plus the separators and null terminator. 2. Updates the snprintf calls to use UBIFS_DFS_DIR_LEN instead of UBIFS_DFS_DIR_LEN + 1, removing the unnecessary +1. 3. Modifies the error checks to compare against UBIFS_DFS_DIR_LEN using >= instead of >, aligning with the corrected macro definition. 4. Removes the redundant +1 in the dfs_dir_name array definitions in ubi.h and debug.h. 5. Renames the duplicated UBIFS_DFS_DIR_LEN and UBIFS_DFS_DIR_NAME macros in the sysfs-related code (sysfs.c and ubifs.h) to UBIFS_SYSFS_DIR_LEN and UBIFS_SYSFS_DIR_NAME, respectively, to differentiate their usage from the debugfs constants. While these changes do not affect the runtime behavior, they make the code more readable, maintainable, and less prone to future errors. Signed-off-by: ZhaoLong Wang <wangzhaolong1@huawei.com> --- drivers/mtd/ubi/debug.c | 4 ++-- drivers/mtd/ubi/ubi.h | 2 +- fs/ubifs/debug.c | 4 ++-- fs/ubifs/debug.h | 7 ++++--- fs/ubifs/sysfs.c | 6 +++--- fs/ubifs/ubifs.h | 7 ++++--- 6 files changed, 16 insertions(+), 14 deletions(-)