Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752686Ab2KECsE (ORCPT ); Sun, 4 Nov 2012 21:48:04 -0500 Received: from mail-ia0-f174.google.com ([209.85.210.174]:47301 "EHLO mail-ia0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751684Ab2KECsC (ORCPT ); Sun, 4 Nov 2012 21:48:02 -0500 MIME-Version: 1.0 In-Reply-To: References: <1351974625-10282-1-git-send-email-Julia.Lawall@lip6.fr> From: Sasha Levin Date: Sun, 4 Nov 2012 21:47:41 -0500 Message-ID: Subject: Re: [PATCH 0/8] drop if around WARN_ON To: Julia Lawall Cc: linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org Content-Type: text/plain; charset=ISO-8859-1 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2015 Lines: 45 On Sun, Nov 4, 2012 at 11:16 AM, Julia Lawall wrote: > I didn't change any cases where the if test contains a function call. The > current definitions of WARN_ON seem to always evaluate the condition > expression, but I was worried that that might not always be the case. And > calling a function (the ones I remember were some kinds of print functions) > seems like something one might not want buried in the argument of a > debugging macro. Makes sense. > WARN_ON_SMP is just WARN_ON if CONFIG_SMP is true, but it is just 0 > otherwise. So in that case it seems important to check that one is not > throwing away something important. Yup, we just need to make sure that the expression being evaluated doesn't have side-effects. > I remember working on the BUG_ON case several years ago, and other people > worked on it too, but I guess some are still there... The current > definitions of BUG_ON seem to keep the condition, but there are quite a few > specialized definitions, so someone at some point might make a version that > does not have that property. It makes sense to keep an eye for such things when converting code. I also don't think we'll get to see a version of BUG_ON which doesn't evaluate the expression since the kernel already has more than enough BUG_ONs that assume the expression will be evaluated: BUG_ON(HYPERVISOR_callback_op(CALLBACKOP_register, &event)); BUG_ON(gpiochip_add(&gemini_gpio_chip)); BUG_ON(clocksource_register_hz(&sirfsoc_clocksource, CLOCK_TICK_RATE)); BUG_ON(gpio_request(ZOOM2_HEADSET_MUX_GPIO, "hs_mux") < 0); And so on, so we're probably safe converting to BUG_ON even if the condition is a function, as long as it doesn't create a long and complicated BUG_ON() ofcourse. Thanks, Sasha -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/