From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 1A2DE3FC8 for ; Sun, 26 Sep 2021 06:51:55 +0000 (UTC) Received: by mail.kernel.org (Postfix) with ESMTPSA id 10DCB60EFF; Sun, 26 Sep 2021 06:51:52 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linuxfoundation.org; s=korg; t=1632639114; bh=gVidkyb0TWHeyFaqlS+HQ7fVRPERCbmqPvMjmlYgrI8=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=U3FbnnrgrbkQnoP7Czee/vvG5Y0NdqEJza6p8WFDSPIgDNdO6FvCOncS9qtts1eTx JcaclcQpOtbZcHMskroBP564psTjCc+EmjDuf6eGgMJxBN6ejsUFPFkOyP/eO2q1zG tP0lLHaAqQAoEf5jDmj2jy6k0HcBYbyS8AFAinZ8= Date: Sun, 26 Sep 2021 08:51:46 +0200 From: Greg Kroah-Hartman To: Michael Estner Cc: Lee Jones , linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org Subject: Re: [PATCH] avoid crashing the kernel Message-ID: References: <20210925200433.8329-1-michaelestner@web.de> Precedence: bulk X-Mailing-List: linux-staging@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210925200433.8329-1-michaelestner@web.de> On Sat, Sep 25, 2021 at 10:04:30PM +0200, Michael Estner wrote: > To avoid chrashing the kernel I use WARN_ON instead. > > Signed-off-by: Michael Estner > --- > drivers/staging/most/i2c/i2c.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) Hi, First off, thanks for the changes, but they need a bit more work. Your subject line should match the others done for this file, so you need a "staging: most:" prefix like others. To see this better, do: git log --oneline the_file_i_am_touching.c Also take a look at the kernel documentation for how to write a good changelog text. Your wording above needs some work... > diff --git a/drivers/staging/most/i2c/i2c.c b/drivers/staging/most/i2c/i2c.c > index 7042f10887bb..e1edd892f9fd 100644 > --- a/drivers/staging/most/i2c/i2c.c > +++ b/drivers/staging/most/i2c/i2c.c > @@ -68,7 +68,7 @@ static int configure_channel(struct most_interface *most_iface, > struct hdm_i2c *dev = to_hdm(most_iface); > unsigned int delay, pr; > > - BUG_ON(ch_idx < 0 || ch_idx >= NUM_CHANNELS); > + WARN_ON(ch_idx < 0 || ch_idx >= NUM_CHANNELS); You really aren't changing much here, for systems running with "panic on warn", right? To solve this correctly you should either: - determine that these are impossible to hit and remove the test entirely - determine that these are possible to hit, and turn them into a real test and handle the error properly. thanks, greg k-h