[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <3fce4fde-0163-4fd4-feb0-2a68aa785e2f@huawei.com>
Date: Mon, 25 Mar 2024 21:46:35 +0800
From: Zhihao Cheng <chengzhihao1@...wei.com>
To: ZhaoLong Wang <wangzhaolong1@...wei.com>, <richard@....at>,
<miquel.raynal@...tlin.com>, <vigneshr@...com>
CC: <linux-mtd@...ts.infradead.org>, <linux-kernel@...r.kernel.org>,
<yi.zhang@...wei.com>, <yangerkun@...wei.com>
Subject: Re: [PATCH V2] ubifs: correct UBIFS_DFS_DIR_LEN macro definition and
improve code clarity
在 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.
>
Powered by blists - more mailing lists