Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755438AbYJVKLQ (ORCPT ); Wed, 22 Oct 2008 06:11:16 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1753180AbYJVKK6 (ORCPT ); Wed, 22 Oct 2008 06:10:58 -0400 Received: from lazybastard.de ([212.112.238.170]:40545 "EHLO longford.logfs.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752954AbYJVKK5 (ORCPT ); Wed, 22 Oct 2008 06:10:57 -0400 Date: Wed, 22 Oct 2008 12:10:13 +0200 From: =?utf-8?B?SsO2cm4=?= Engel To: Ingo Molnar Cc: Linus Torvalds , Roland Dreier , Andrew Morton , "David S. Miller" , Alan Cox , linux-kernel@vger.kernel.org, Peter Zijlstra , Thomas Gleixner , "H. Peter Anvin" , David Howells Subject: Re: [announce] new tree: "fix all build warnings, on all configs" Message-ID: <20081022101013.GA25283@logfs.org> References: <20081017180523.GA11590@elte.hu> <20081017191202.GA5396@elte.hu> <20081018082209.GA24220@elte.hu> <20081020192110.GA28736@elte.hu> <20081021064144.GC30563@logfs.org> <20081022094759.GH12453@elte.hu> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20081022094759.GH12453@elte.hu> User-Agent: Mutt/1.5.13 (2006-08-11) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1954 Lines: 45 On Wed, 22 October 2008 11:47:59 +0200, Ingo Molnar wrote: > * Jörn Engel wrote: > > On Mon, 20 October 2008 21:21:10 +0200, Ingo Molnar wrote: > > > > > > /* Alas, no aliases. Too much hassle with bringing module.h everywhere */ > > > #define fops_get(fops) \ > > > - (((fops) && try_module_get((fops)->owner) ? (fops) : NULL)) > > > + (((fops != NULL) && try_module_get((fops)->owner) ? (fops) : NULL)) > > > #define fops_put(fops) \ > > > - do { if (fops) module_put((fops)->owner); } while(0) > > > + do { if (fops != NULL) module_put((fops)->owner); } while(0) > > > > This, I would argue, makes the code worse. > > In this specific case the issue is that the 'fops' parameter can > occasionally be a constant pointer (turning the test into always-true) > so the compiler is at least minimally correct at asking the "are you > sure you want this" question - which we answer in the affirmative via > the explicit NULL check. But these are really nuances. #define fops_put(fops) do { \ if (fops != NULL) \ module_put((fops)->owner); \ } while 0 If the code was written like this, I wouldn't mind your patch at all. But in the one-liner form, the ' != NULL' makes the difference between fluent reading and having to actively sort out which piece belongs where. And I bet that if it hadn't been a macro but an inline function, you and checkpatch would have complained about the formatting long ago. ;) Jörn -- To announce that there must be no criticism of the President, or that we are to stand by the President, right or wrong, is not only unpatriotic and servile, but is morally treasonable to the American public. -- Theodore Roosevelt, Kansas City Star, 1918 -- 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/