From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-ot1-f50.google.com (mail-ot1-f50.google.com [209.85.210.50]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 924DB158D80 for ; Fri, 3 Jan 2025 03:13:10 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.210.50 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1735873992; cv=none; b=A5RKGaZc3wwf1Ks6CDnHkpMgBXfybOqSubkfxp/jrqe3Gt9wwyRpYLQ6N42sNtsYZH4a9coxAjCK4NwBjwxmrDeXfdnkTgf7yyhhskn3B24fy9Ik0bzbZMjmAE2ipjp1MtBDAETFmTevgGyGmBWaDckWdXoBH/kG0UHUQWl2uRw= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1735873992; c=relaxed/simple; bh=Zx8KSL4aPToMEWDjqnu1nlYyxS6f6zyPCNjiUI4NbJc=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=GVbI2YrOOjaGRhwTb9o72dyt8o3H9HxJBhQrbFeuflHoWe4KXk4NPo0kl6CFnOQn241GV7yAgMLcUrzmrZjh88TkYLo9dUF4EMJ66VL5wGTWBjcPSjA/9LfBfQ6rgdJlKtduztBfZ+oQjy7wQz0kWCyxAfxC9ShEwBNtQckh4o0= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=minyard.net; spf=none smtp.mailfrom=minyard.net; dkim=pass (2048-bit key) header.d=minyard-net.20230601.gappssmtp.com header.i=@minyard-net.20230601.gappssmtp.com header.b=N1UCFEne; arc=none smtp.client-ip=209.85.210.50 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=minyard.net Authentication-Results: smtp.subspace.kernel.org; spf=none smtp.mailfrom=minyard.net Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=minyard-net.20230601.gappssmtp.com header.i=@minyard-net.20230601.gappssmtp.com header.b="N1UCFEne" Received: by mail-ot1-f50.google.com with SMTP id 46e09a7af769-71e3cbd0583so2614827a34.1 for ; Thu, 02 Jan 2025 19:13:10 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=minyard-net.20230601.gappssmtp.com; s=20230601; t=1735873989; x=1736478789; darn=vger.kernel.org; h=in-reply-to:content-disposition:mime-version:references:reply-to :message-id:subject:cc:to:from:date:from:to:cc:subject:date :message-id:reply-to; bh=ojyDsvzcYX6bZ9hy4t2yQkdLV82UeQ9UgOyBYBPPp1A=; b=N1UCFEneIyx7cmM/kxzsSEb5d5BsXgwOOxIu7QTGB8TR0m9Xma0oqfgSPQdSOFqoMP PnYMphditFHSmarultlXUaKTPKywDwEa8yooXNLECSRtq+eETUiPGyEIzFw/4MdhruIN WKQ2oFsqu+FoiyrYUBfpKzc3Pka9WGo4BtX9I5x/51d/pC7Zp+clLGKuAaeLdrwO8byE 4ZFtvn6pYfOxxNsS3hqK077MYlCQhDBzDm3HXs4p59NmmGp3aBqCVJzXlhEya12j275i EQ1c3O5In2mXALXvzwNnmQi206buXnbXqWc/2Glk63Qwug1z0TkXA7x8A5gvnjhMGe4d WssQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1735873989; x=1736478789; h=in-reply-to:content-disposition:mime-version:references:reply-to :message-id:subject:cc:to:from:date:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=ojyDsvzcYX6bZ9hy4t2yQkdLV82UeQ9UgOyBYBPPp1A=; b=TuRYwsUEKUn9IEm8ks3MbUHpRtLr90aWtx6Zftw7y5iR8bTJmFKFXzTTDSqIXcP57u nbc0YEg0Y4/U4nObZ1FErcDAnAgNkAAbcJ45+dLWHxSPywnM59WS+X0Y673w57KwqQfA plFaMtJVGzfchg4lsJ8r854aGueyhY6Jpczlu4XRhx3MH2v9Vsqkt2F9d9dd54UtknZA C50dpDky8QQqKsvmJX6hC8T/VGPtE9/PEa38qzyEZoSaTM+67Q0Wu8dErnpvjc+BWYLv K225hxFNs6CUr/PXoX69KH5UUwcqRQpHOyw2yyRJtamICoqHJAToQjcL+JTVg5aTImx9 CVZw== X-Forwarded-Encrypted: i=1; AJvYcCUEpHJI6vmbzbwvGnWhm3Byh7YoMGuTvcI9k9rngUc2KVtCVrLfQ0v4oi35qCIZpuQsLOjUOpXrg1DjEEQ=@vger.kernel.org X-Gm-Message-State: AOJu0Yyj04IBlIjj9GSgoMc+yki7ABFtkMxBIZB9SKLhMCP89nzM0GCy gW4XVX3M8iZ1sMFfhETETspj4qRo5VjRV+UnFOznBOBd/UxKlwDlrgjBfLfk1og= X-Gm-Gg: ASbGnctQDAl8N3EL6y4ampIXOlllg1hIF4jfAniBpPqOOSg0l/3urE1bO3ZY03oUM/R T+BXcBkHyK92NVyM1A2kSL3Jt6I9Nzl/W4coJaDYY+Vw1MiLPVm67BP+dinjqRSX87eoncSzlMj mLMVxiWOXzW3wFm0kXTrA4c2w13idOaIg6cLqM6NYZR6epZU05rQ+Wxm2Lx3V7jAzmVGGBMEMs5 vgpb9kqSB6YnQjkzwd20tqyWztXOpTOQ6pcuVmmWY3Sslmdrnku6fpZVTTW X-Google-Smtp-Source: AGHT+IEslTTiif85XWGg8q4fNEh33Y6f4ykrORDcDRtaao/HSQsmL91nYJNrhpwMcqyeh0FHcF+sHg== X-Received: by 2002:a05:6830:6f43:b0:71d:88f0:b13 with SMTP id 46e09a7af769-720ff6850dcmr26206959a34.1.1735873989618; Thu, 02 Jan 2025 19:13:09 -0800 (PST) Received: from mail.minyard.net ([2001:470:b8f6:1b:81ab:b2d6:d879:cada]) by smtp.gmail.com with ESMTPSA id 46e09a7af769-71fc97a4085sm8072576a34.22.2025.01.02.19.13.06 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 02 Jan 2025 19:13:08 -0800 (PST) Date: Thu, 2 Jan 2025 21:13:03 -0600 From: Corey Minyard To: Vitaliy Shevtsov Cc: Corey Minyard , openipmi-developer@lists.sourceforge.net, linux-kernel@vger.kernel.org, lvc-project@linuxtesting.org Subject: Re: [PATCH v2] ipmi: make ipmi_destroy_user() return void Message-ID: Reply-To: corey@minyard.net References: <20241225014532.20091-1-v.shevtsov@maxima.ru> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20241225014532.20091-1-v.shevtsov@maxima.ru> 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 > --- > 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 >