* [PATCH] mmc: core: Fix error checking
@ 2023-05-17 19:26 Yeqi Fu
2023-05-18 8:15 ` Adrian Hunter
0 siblings, 1 reply; 5+ messages in thread
From: Yeqi Fu @ 2023-05-17 19:26 UTC (permalink / raw
To: ulf.hansson, CLoehle, adrian.hunter, avri.altman, axboe
Cc: Yeqi Fu, linux-mmc, linux-kernel, Ivan Orlov
The functions debugfs_create_dir and debugfs_create_file_unsafe return
ERR_PTR if an error occurs, and the appropriate way to verify for errors
is to use the inline function IS_ERR. The patch will substitute the
null-comparison with IS_ERR.
Suggested-by: Ivan Orlov <ivan.orlov0322@gmail.com>
Signed-off-by: Yeqi Fu <asuk4.q@gmail.com>
---
drivers/mmc/core/block.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
index 00c33edb9fb9..507bebc22636 100644
--- a/drivers/mmc/core/block.c
+++ b/drivers/mmc/core/block.c
@@ -2908,7 +2908,7 @@ static int mmc_blk_add_debugfs(struct mmc_card *card, struct mmc_blk_data *md)
debugfs_create_file_unsafe("status", 0400, root,
card,
&mmc_dbg_card_status_fops);
- if (!md->status_dentry)
+ if (IS_ERR(md->status_dentry))
return -EIO;
}
@@ -2916,7 +2916,7 @@ static int mmc_blk_add_debugfs(struct mmc_card *card, struct mmc_blk_data *md)
md->ext_csd_dentry =
debugfs_create_file("ext_csd", S_IRUSR, root, card,
&mmc_dbg_ext_csd_fops);
- if (!md->ext_csd_dentry)
+ if (IS_ERR(md->ext_csd_dentry))
return -EIO;
}
--
2.37.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] mmc: core: Fix error checking
2023-05-17 19:26 [PATCH] mmc: core: Fix error checking Yeqi Fu
@ 2023-05-18 8:15 ` Adrian Hunter
2023-05-18 10:12 ` [PATCH] mmc: core: Remove unnecessary error checks and change return type Yeqi Fu
0 siblings, 1 reply; 5+ messages in thread
From: Adrian Hunter @ 2023-05-18 8:15 UTC (permalink / raw
To: Yeqi Fu, ulf.hansson, CLoehle, avri.altman, axboe
Cc: linux-mmc, linux-kernel, Ivan Orlov
On 17/05/23 22:26, Yeqi Fu wrote:
> The functions debugfs_create_dir and debugfs_create_file_unsafe return
> ERR_PTR if an error occurs, and the appropriate way to verify for errors
> is to use the inline function IS_ERR. The patch will substitute the
> null-comparison with IS_ERR.
>
> Suggested-by: Ivan Orlov <ivan.orlov0322@gmail.com>
> Signed-off-by: Yeqi Fu <asuk4.q@gmail.com>
> ---
> drivers/mmc/core/block.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
> index 00c33edb9fb9..507bebc22636 100644
> --- a/drivers/mmc/core/block.c
> +++ b/drivers/mmc/core/block.c
> @@ -2908,7 +2908,7 @@ static int mmc_blk_add_debugfs(struct mmc_card *card, struct mmc_blk_data *md)
> debugfs_create_file_unsafe("status", 0400, root,
> card,
> &mmc_dbg_card_status_fops);
> - if (!md->status_dentry)
> + if (IS_ERR(md->status_dentry))
> return -EIO;
> }
>
> @@ -2916,7 +2916,7 @@ static int mmc_blk_add_debugfs(struct mmc_card *card, struct mmc_blk_data *md)
> md->ext_csd_dentry =
> debugfs_create_file("ext_csd", S_IRUSR, root, card,
> &mmc_dbg_ext_csd_fops);
> - if (!md->ext_csd_dentry)
> + if (IS_ERR(md->ext_csd_dentry))
> return -EIO;
> }
>
The patch is not wrong, but you also need to look at the bigger picture.
In this case, the return value is not used. And debugfs API is designed
so that return values can be ignored - for example, it is ok to pass NULL
or an error code to debugfs_remove(). Generally we don't care if debugfs
fails, because it is only for debugging, but it only uses memory resources
so it essentially doesn't fail anyway - except when it is not compiled in.
So you could change mmc_blk_add_debugfs() to return void, and drop the error
checks entirely.
The error checks in mmc_blk_remove_debugfs() also serve no purpose.
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] mmc: core: Remove unnecessary error checks and change return type
2023-05-18 8:15 ` Adrian Hunter
@ 2023-05-18 10:12 ` Yeqi Fu
2023-05-18 16:43 ` Adrian Hunter
2023-05-24 13:10 ` Ulf Hansson
0 siblings, 2 replies; 5+ messages in thread
From: Yeqi Fu @ 2023-05-18 10:12 UTC (permalink / raw
To: ulf.hansson, CLoehle, adrian.hunter, avri.altman, axboe
Cc: Yeqi Fu, linux-mmc, linux-kernel
The error checks in mmc_blk_add_debugfs() and mmc_blk_remove_debugfs()
are extraneous. Therefore, this patch removes all error checks from
both functions.
Additionally, mmc_blk_add_debugfs() has been changed to return void
instead of an integer value that was never used. This simplifies the
function and improves its clarity.
Signed-off-by: Yeqi Fu <asuk4.q@gmail.com>
---
drivers/mmc/core/block.c | 25 +++++++------------------
1 file changed, 7 insertions(+), 18 deletions(-)
diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
index 00c33edb9fb9..81f33200b893 100644
--- a/drivers/mmc/core/block.c
+++ b/drivers/mmc/core/block.c
@@ -2894,12 +2894,12 @@ static const struct file_operations mmc_dbg_ext_csd_fops = {
.llseek = default_llseek,
};
-static int mmc_blk_add_debugfs(struct mmc_card *card, struct mmc_blk_data *md)
+static void mmc_blk_add_debugfs(struct mmc_card *card, struct mmc_blk_data *md)
{
struct dentry *root;
if (!card->debugfs_root)
- return 0;
+ return;
root = card->debugfs_root;
@@ -2908,19 +2908,13 @@ static int mmc_blk_add_debugfs(struct mmc_card *card, struct mmc_blk_data *md)
debugfs_create_file_unsafe("status", 0400, root,
card,
&mmc_dbg_card_status_fops);
- if (!md->status_dentry)
- return -EIO;
}
if (mmc_card_mmc(card)) {
md->ext_csd_dentry =
debugfs_create_file("ext_csd", S_IRUSR, root, card,
&mmc_dbg_ext_csd_fops);
- if (!md->ext_csd_dentry)
- return -EIO;
}
-
- return 0;
}
static void mmc_blk_remove_debugfs(struct mmc_card *card,
@@ -2929,22 +2923,17 @@ static void mmc_blk_remove_debugfs(struct mmc_card *card,
if (!card->debugfs_root)
return;
- if (!IS_ERR_OR_NULL(md->status_dentry)) {
- debugfs_remove(md->status_dentry);
- md->status_dentry = NULL;
- }
+ debugfs_remove(md->status_dentry);
+ md->status_dentry = NULL;
- if (!IS_ERR_OR_NULL(md->ext_csd_dentry)) {
- debugfs_remove(md->ext_csd_dentry);
- md->ext_csd_dentry = NULL;
- }
+ debugfs_remove(md->ext_csd_dentry);
+ md->ext_csd_dentry = NULL;
}
#else
-static int mmc_blk_add_debugfs(struct mmc_card *card, struct mmc_blk_data *md)
+static void mmc_blk_add_debugfs(struct mmc_card *card, struct mmc_blk_data *md)
{
- return 0;
}
static void mmc_blk_remove_debugfs(struct mmc_card *card,
--
2.37.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] mmc: core: Remove unnecessary error checks and change return type
2023-05-18 10:12 ` [PATCH] mmc: core: Remove unnecessary error checks and change return type Yeqi Fu
@ 2023-05-18 16:43 ` Adrian Hunter
2023-05-24 13:10 ` Ulf Hansson
1 sibling, 0 replies; 5+ messages in thread
From: Adrian Hunter @ 2023-05-18 16:43 UTC (permalink / raw
To: Yeqi Fu, ulf.hansson, CLoehle, avri.altman, axboe; +Cc: linux-mmc, linux-kernel
On 18/05/23 13:12, Yeqi Fu wrote:
> The error checks in mmc_blk_add_debugfs() and mmc_blk_remove_debugfs()
> are extraneous. Therefore, this patch removes all error checks from
> both functions.
> Additionally, mmc_blk_add_debugfs() has been changed to return void
> instead of an integer value that was never used. This simplifies the
> function and improves its clarity.
>
> Signed-off-by: Yeqi Fu <asuk4.q@gmail.com>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
> drivers/mmc/core/block.c | 25 +++++++------------------
> 1 file changed, 7 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
> index 00c33edb9fb9..81f33200b893 100644
> --- a/drivers/mmc/core/block.c
> +++ b/drivers/mmc/core/block.c
> @@ -2894,12 +2894,12 @@ static const struct file_operations mmc_dbg_ext_csd_fops = {
> .llseek = default_llseek,
> };
>
> -static int mmc_blk_add_debugfs(struct mmc_card *card, struct mmc_blk_data *md)
> +static void mmc_blk_add_debugfs(struct mmc_card *card, struct mmc_blk_data *md)
> {
> struct dentry *root;
>
> if (!card->debugfs_root)
> - return 0;
> + return;
>
> root = card->debugfs_root;
>
> @@ -2908,19 +2908,13 @@ static int mmc_blk_add_debugfs(struct mmc_card *card, struct mmc_blk_data *md)
> debugfs_create_file_unsafe("status", 0400, root,
> card,
> &mmc_dbg_card_status_fops);
> - if (!md->status_dentry)
> - return -EIO;
> }
>
> if (mmc_card_mmc(card)) {
> md->ext_csd_dentry =
> debugfs_create_file("ext_csd", S_IRUSR, root, card,
> &mmc_dbg_ext_csd_fops);
> - if (!md->ext_csd_dentry)
> - return -EIO;
> }
> -
> - return 0;
> }
>
> static void mmc_blk_remove_debugfs(struct mmc_card *card,
> @@ -2929,22 +2923,17 @@ static void mmc_blk_remove_debugfs(struct mmc_card *card,
> if (!card->debugfs_root)
> return;
>
> - if (!IS_ERR_OR_NULL(md->status_dentry)) {
> - debugfs_remove(md->status_dentry);
> - md->status_dentry = NULL;
> - }
> + debugfs_remove(md->status_dentry);
> + md->status_dentry = NULL;
>
> - if (!IS_ERR_OR_NULL(md->ext_csd_dentry)) {
> - debugfs_remove(md->ext_csd_dentry);
> - md->ext_csd_dentry = NULL;
> - }
> + debugfs_remove(md->ext_csd_dentry);
> + md->ext_csd_dentry = NULL;
> }
>
> #else
>
> -static int mmc_blk_add_debugfs(struct mmc_card *card, struct mmc_blk_data *md)
> +static void mmc_blk_add_debugfs(struct mmc_card *card, struct mmc_blk_data *md)
> {
> - return 0;
> }
>
> static void mmc_blk_remove_debugfs(struct mmc_card *card,
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] mmc: core: Remove unnecessary error checks and change return type
2023-05-18 10:12 ` [PATCH] mmc: core: Remove unnecessary error checks and change return type Yeqi Fu
2023-05-18 16:43 ` Adrian Hunter
@ 2023-05-24 13:10 ` Ulf Hansson
1 sibling, 0 replies; 5+ messages in thread
From: Ulf Hansson @ 2023-05-24 13:10 UTC (permalink / raw
To: Yeqi Fu; +Cc: CLoehle, adrian.hunter, avri.altman, axboe, linux-mmc,
linux-kernel
On Thu, 18 May 2023 at 12:13, Yeqi Fu <asuk4.q@gmail.com> wrote:
>
> The error checks in mmc_blk_add_debugfs() and mmc_blk_remove_debugfs()
> are extraneous. Therefore, this patch removes all error checks from
> both functions.
> Additionally, mmc_blk_add_debugfs() has been changed to return void
> instead of an integer value that was never used. This simplifies the
> function and improves its clarity.
>
> Signed-off-by: Yeqi Fu <asuk4.q@gmail.com>
Applied for next, thanks!
Kind regards
Uffe
> ---
> drivers/mmc/core/block.c | 25 +++++++------------------
> 1 file changed, 7 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
> index 00c33edb9fb9..81f33200b893 100644
> --- a/drivers/mmc/core/block.c
> +++ b/drivers/mmc/core/block.c
> @@ -2894,12 +2894,12 @@ static const struct file_operations mmc_dbg_ext_csd_fops = {
> .llseek = default_llseek,
> };
>
> -static int mmc_blk_add_debugfs(struct mmc_card *card, struct mmc_blk_data *md)
> +static void mmc_blk_add_debugfs(struct mmc_card *card, struct mmc_blk_data *md)
> {
> struct dentry *root;
>
> if (!card->debugfs_root)
> - return 0;
> + return;
>
> root = card->debugfs_root;
>
> @@ -2908,19 +2908,13 @@ static int mmc_blk_add_debugfs(struct mmc_card *card, struct mmc_blk_data *md)
> debugfs_create_file_unsafe("status", 0400, root,
> card,
> &mmc_dbg_card_status_fops);
> - if (!md->status_dentry)
> - return -EIO;
> }
>
> if (mmc_card_mmc(card)) {
> md->ext_csd_dentry =
> debugfs_create_file("ext_csd", S_IRUSR, root, card,
> &mmc_dbg_ext_csd_fops);
> - if (!md->ext_csd_dentry)
> - return -EIO;
> }
> -
> - return 0;
> }
>
> static void mmc_blk_remove_debugfs(struct mmc_card *card,
> @@ -2929,22 +2923,17 @@ static void mmc_blk_remove_debugfs(struct mmc_card *card,
> if (!card->debugfs_root)
> return;
>
> - if (!IS_ERR_OR_NULL(md->status_dentry)) {
> - debugfs_remove(md->status_dentry);
> - md->status_dentry = NULL;
> - }
> + debugfs_remove(md->status_dentry);
> + md->status_dentry = NULL;
>
> - if (!IS_ERR_OR_NULL(md->ext_csd_dentry)) {
> - debugfs_remove(md->ext_csd_dentry);
> - md->ext_csd_dentry = NULL;
> - }
> + debugfs_remove(md->ext_csd_dentry);
> + md->ext_csd_dentry = NULL;
> }
>
> #else
>
> -static int mmc_blk_add_debugfs(struct mmc_card *card, struct mmc_blk_data *md)
> +static void mmc_blk_add_debugfs(struct mmc_card *card, struct mmc_blk_data *md)
> {
> - return 0;
> }
>
> static void mmc_blk_remove_debugfs(struct mmc_card *card,
> --
> 2.37.2
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-05-24 13:12 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-05-17 19:26 [PATCH] mmc: core: Fix error checking Yeqi Fu
2023-05-18 8:15 ` Adrian Hunter
2023-05-18 10:12 ` [PATCH] mmc: core: Remove unnecessary error checks and change return type Yeqi Fu
2023-05-18 16:43 ` Adrian Hunter
2023-05-24 13:10 ` Ulf Hansson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).