[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <Z3dVv-SYIljxw_OX@mail.minyard.net>
Date: Thu, 2 Jan 2025 21:13:03 -0600
From: Corey Minyard <corey@...yard.net>
To: Vitaliy Shevtsov <v.shevtsov@...ima.ru>
Cc: Corey Minyard <minyard@....org>,
openipmi-developer@...ts.sourceforge.net,
linux-kernel@...r.kernel.org, lvc-project@...uxtesting.org
Subject: Re: [PATCH v2] ipmi: make ipmi_destroy_user() return void
On Wed, Dec 25, 2024 at 01:45:30AM +0000, Vitaliy Shevtsov wrote:
> Return value of ipmi_destroy_user() has no meaning, because it's always
> zero and callers can do nothing with it. And in most cases it's not
> checked. So make this function return void. This also will eliminate static
> code analyzer warnings such as unreachable code/redundant comparison when
> the return value is checked against non-zero value.
This is applied to my next tree, thank you.
-corey
>
> Found by Linux Verification Center (linuxtesting.org) with Svace.
>
> Signed-off-by: Vitaliy Shevtsov <v.shevtsov@...ima.ru>
> ---
> v2: Add changes in drivers/char/ipmi/ipmi_poweroff.c missed by chance
>
> drivers/char/ipmi/ipmi_devintf.c | 5 +----
> drivers/char/ipmi/ipmi_msghandler.c | 4 +---
> drivers/char/ipmi/ipmi_poweroff.c | 6 +-----
> drivers/char/ipmi/ipmi_watchdog.c | 5 +----
> include/linux/ipmi.h | 2 +-
> 5 files changed, 5 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/char/ipmi/ipmi_devintf.c b/drivers/char/ipmi/ipmi_devintf.c
> index 332082e02ea5..e6ba35b71f10 100644
> --- a/drivers/char/ipmi/ipmi_devintf.c
> +++ b/drivers/char/ipmi/ipmi_devintf.c
> @@ -122,12 +122,9 @@ static int ipmi_open(struct inode *inode, struct file *file)
> static int ipmi_release(struct inode *inode, struct file *file)
> {
> struct ipmi_file_private *priv = file->private_data;
> - int rv;
> struct ipmi_recv_msg *msg, *next;
>
> - rv = ipmi_destroy_user(priv->user);
> - if (rv)
> - return rv;
> + ipmi_destroy_user(priv->user);
>
> list_for_each_entry_safe(msg, next, &priv->recv_msgs, link)
> ipmi_free_recv_msg(msg);
> diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c
> index e12b531f5c2f..1e5313748f8b 100644
> --- a/drivers/char/ipmi/ipmi_msghandler.c
> +++ b/drivers/char/ipmi/ipmi_msghandler.c
> @@ -1398,13 +1398,11 @@ static void _ipmi_destroy_user(struct ipmi_user *user)
> module_put(owner);
> }
>
> -int ipmi_destroy_user(struct ipmi_user *user)
> +void ipmi_destroy_user(struct ipmi_user *user)
> {
> _ipmi_destroy_user(user);
>
> kref_put(&user->refcount, free_user);
> -
> - return 0;
> }
> EXPORT_SYMBOL(ipmi_destroy_user);
>
> diff --git a/drivers/char/ipmi/ipmi_poweroff.c b/drivers/char/ipmi/ipmi_poweroff.c
> index 941d2dcc8c9d..05f17e3e6207 100644
> --- a/drivers/char/ipmi/ipmi_poweroff.c
> +++ b/drivers/char/ipmi/ipmi_poweroff.c
> @@ -699,8 +699,6 @@ static int __init ipmi_poweroff_init(void)
> #ifdef MODULE
> static void __exit ipmi_poweroff_cleanup(void)
> {
> - int rv;
> -
> #ifdef CONFIG_PROC_FS
> unregister_sysctl_table(ipmi_table_header);
> #endif
> @@ -708,9 +706,7 @@ static void __exit ipmi_poweroff_cleanup(void)
> ipmi_smi_watcher_unregister(&smi_watcher);
>
> if (ready) {
> - rv = ipmi_destroy_user(ipmi_user);
> - if (rv)
> - pr_err("could not cleanup the IPMI user: 0x%x\n", rv);
> + ipmi_destroy_user(ipmi_user);
> pm_power_off = old_poweroff_func;
> }
> }
> diff --git a/drivers/char/ipmi/ipmi_watchdog.c b/drivers/char/ipmi/ipmi_watchdog.c
> index 335eea80054e..f1875b2bebbc 100644
> --- a/drivers/char/ipmi/ipmi_watchdog.c
> +++ b/drivers/char/ipmi/ipmi_watchdog.c
> @@ -1064,7 +1064,6 @@ static void ipmi_register_watchdog(int ipmi_intf)
>
> static void ipmi_unregister_watchdog(int ipmi_intf)
> {
> - int rv;
> struct ipmi_user *loc_user = watchdog_user;
>
> if (!loc_user)
> @@ -1089,9 +1088,7 @@ static void ipmi_unregister_watchdog(int ipmi_intf)
> mutex_lock(&ipmi_watchdog_mutex);
>
> /* Disconnect from IPMI. */
> - rv = ipmi_destroy_user(loc_user);
> - if (rv)
> - pr_warn("error unlinking from IPMI: %d\n", rv);
> + ipmi_destroy_user(loc_user);
>
> /* If it comes back, restart it properly. */
> ipmi_start_timer_on_heartbeat = 1;
> diff --git a/include/linux/ipmi.h b/include/linux/ipmi.h
> index a1c9c0d48ebf..2f74dd90c271 100644
> --- a/include/linux/ipmi.h
> +++ b/include/linux/ipmi.h
> @@ -126,7 +126,7 @@ int ipmi_create_user(unsigned int if_num,
> * the users before you destroy the callback structures, it should be
> * safe, too.
> */
> -int ipmi_destroy_user(struct ipmi_user *user);
> +void ipmi_destroy_user(struct ipmi_user *user);
>
> /* Get the IPMI version of the BMC we are talking to. */
> int ipmi_get_version(struct ipmi_user *user,
> --
> 2.47.1
>
Powered by blists - more mailing lists