From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Jinjie Ruan <ruanjinjie@huawei.com>
Cc: linux-phy@lists.infradead.org, Vinod Koul <vkoul@kernel.org>,
Kishon Vijay Abraham I <kishon@kernel.org>,
Stanley Chang <stanley_chang@realtek.com>
Subject: Re: [PATCH] phy: realtek: usb: Fix the NULL vs IS_ERR() bug for debugfs_create_dir()
Date: Thu, 31 Aug 2023 16:39:31 +0200 [thread overview]
Message-ID: <2023083101-jester-trillion-ba31@gregkh> (raw)
In-Reply-To: <20230831140559.3166158-1-ruanjinjie@huawei.com>
On Thu, Aug 31, 2023 at 10:05:59PM +0800, Jinjie Ruan wrote:
> Since both debugfs_create_dir() and debugfs_create_file() return
> ERR_PTR and never return NULL, So use IS_ERR() to check it
> instead of checking NULL.
>
> Fixes: 134e6d25f6bd ("phy: realtek: usb: Add driver for the Realtek SoC USB 2.0 PHY")
> Fixes: adda6e82a7de ("phy: realtek: usb: Add driver for the Realtek SoC USB 3.0 PHY")
> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
> ---
> drivers/phy/realtek/phy-rtk-usb2.c | 6 +++---
> drivers/phy/realtek/phy-rtk-usb3.c | 6 +++---
> 2 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/phy/realtek/phy-rtk-usb2.c b/drivers/phy/realtek/phy-rtk-usb2.c
> index 5e7ee060b404..67ca78762c80 100644
> --- a/drivers/phy/realtek/phy-rtk-usb2.c
> +++ b/drivers/phy/realtek/phy-rtk-usb2.c
> @@ -853,11 +853,11 @@ static inline void create_debug_files(struct rtk_phy *rtk_phy)
>
> rtk_phy->debug_dir = debugfs_create_dir(dev_name(rtk_phy->dev),
> phy_debug_root);
> - if (!rtk_phy->debug_dir)
> + if (IS_ERR(rtk_phy->debug_dir))
> return;
No, please just drop this check.
>
> - if (!debugfs_create_file("parameter", 0444, rtk_phy->debug_dir, rtk_phy,
> - &rtk_usb2_parameter_fops))
> + if (IS_ERR(debugfs_create_file("parameter", 0444, rtk_phy->debug_dir, rtk_phy,
> + &rtk_usb2_parameter_fops)))
Again, no, please never check the return value of debugfs_create_file(),
it's not needed at all, and no kernel code should ever do something
different if debugfs returns an error.
debugfs calls should not be checked, they can just be safely passed back
into other debugfs calls, if needed. That is it. This api is supposed
to be simple and safe, please don't make it harder than it needs to be :)
thanks,
greg k-h
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
prev parent reply other threads:[~2023-08-31 14:39 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-08-31 14:05 [PATCH] phy: realtek: usb: Fix the NULL vs IS_ERR() bug for debugfs_create_dir() Jinjie Ruan
2023-08-31 14:39 ` Greg Kroah-Hartman [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2023083101-jester-trillion-ba31@gregkh \
--to=gregkh@linuxfoundation.org \
--cc=kishon@kernel.org \
--cc=linux-phy@lists.infradead.org \
--cc=ruanjinjie@huawei.com \
--cc=stanley_chang@realtek.com \
--cc=vkoul@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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 NNTP newsgroup(s).