Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752172AbdFUSd0 convert rfc822-to-8bit (ORCPT ); Wed, 21 Jun 2017 14:33:26 -0400 Received: from youngberry.canonical.com ([91.189.89.112]:50882 "EHLO youngberry.canonical.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751191AbdFUSdY (ORCPT ); Wed, 21 Jun 2017 14:33:24 -0400 From: Jay Vosburgh To: Michael J Dilmore cc: vfalico@gmail.com, andy@greyhouse.net, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, joe@perches.com Subject: Re: [PATCH] Convert BUG_ON to WARN_ON in bond_options.c In-reply-to: <1498068137-3550-1-git-send-email-michael.j.dilmore@gmail.com> References: <1498068137-3550-1-git-send-email-michael.j.dilmore@gmail.com> Comments: In-reply-to Michael J Dilmore message dated "Wed, 21 Jun 2017 19:02:17 +0100." X-Mailer: MH-E 8.6+git-jv; nmh 1.6; GNU Emacs 25.1.50 MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-ID: <21643.1498070000.1@famine> Content-Transfer-Encoding: 8BIT Date: Wed, 21 Jun 2017 11:33:20 -0700 Message-ID: <21644.1498070000@famine> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1147 Lines: 35 Michael J Dilmore wrote: >The function below contains a BUG_ON where no active slave is detected. The patch >converts this to a WARN_ON to avoid crashing the kernel. > >Signed-off-by: Michael J Dilmore >--- > drivers/net/bonding/bond_options.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > >diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c >index 1bcbb89..c4b4791 100644 >--- a/drivers/net/bonding/bond_options.c >+++ b/drivers/net/bonding/bond_options.c >@@ -778,7 +778,7 @@ static int bond_option_active_slave_set(struct bonding *bond, > struct slave *old_active = rtnl_dereference(bond->curr_active_slave); > struct slave *new_active = bond_slave_get_rtnl(slave_dev); > >- BUG_ON(!new_active); >+ WARN_ON(!new_active); This is a reasonable idea in principle, but will require additional changes to prevent dereferencing new_active if it is NULL (which would happen just below this point in the code). -J > if (new_active == old_active) { > /* do nothing */ >-- >2.7.4 > --- -Jay Vosburgh, jay.vosburgh@canonical.com