From owner-freebsd-current@FreeBSD.ORG Mon Jun 15 19:51:56 2009 Return-Path: Delivered-To: current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id BC933106566C; Mon, 15 Jun 2009 19:51:56 +0000 (UTC) (envelope-from sam@freebsd.org) Received: from ebb.errno.com (ebb.errno.com [69.12.149.25]) by mx1.freebsd.org (Postfix) with ESMTP id 8E38D8FC1C; Mon, 15 Jun 2009 19:51:56 +0000 (UTC) (envelope-from sam@freebsd.org) Received: from trouble.errno.com (trouble.errno.com [10.0.0.248]) (authenticated bits=0) by ebb.errno.com (8.13.6/8.12.6) with ESMTP id n5FJptTa065459 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Mon, 15 Jun 2009 12:51:55 -0700 (PDT) (envelope-from sam@freebsd.org) Message-ID: <4A36A65B.9050706@freebsd.org> Date: Mon, 15 Jun 2009 12:51:55 -0700 From: Sam Leffler Organization: FreeBSD Project User-Agent: Thunderbird 2.0.0.21 (X11/20090411) MIME-Version: 1.0 To: Marcel Moolenaar References: <20090615181555.GA52009@freebsd.org> <4A369529.5090004@freebsd.org> <20090615185812.GA67104@freebsd.org> <53E03A0A-8846-4EED-AE95-A15960FC6724@mac.com> In-Reply-To: <53E03A0A-8846-4EED-AE95-A15960FC6724@mac.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-DCC-Misty-Metrics: ebb.errno.com; whitelist Cc: Roman Divacky , current@freebsd.org Subject: Re: [RFC]: (void)0 instead of empty defines X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 15 Jun 2009 19:51:57 -0000 Marcel Moolenaar wrote: > > On Jun 15, 2009, at 11:58 AM, Roman Divacky wrote: > >>> Are you saying that: >>> >>> if (cond) >>> ; >>> >>> is considered worthy of a warning by the compiler? Is it just "if" or >>> all conditional control constructs (e.g. while)? >>> >>> I can image many instances of this construct arising from debugging >>> facilities. This sounds like a stupid restriction and I would argue we >>> should just disable the warning. >> >> it already found a bug in csup (recently fixed by lulf). It sure can be >> disabled but I'd like it to be discussed a little bit more as it already >> proved to be useful. > > If the patch is all we need to compile the kernel with the warning > enabled and knowing that the warning has already found real bugs, > then it's a no-brainer to me: commit. > The patch is incomplete in that it only changes code that makes clang kvetch. There are many other instances in the tree of using #defines for debug facilities and leaving them empty when disabled that this patch does not address. What this change effectively does is impose a new element of style to deal w/ a compiler idiosynchrocy (and/or compilation option). While I can agree finding bugs is good I don't see this as a reason to add this requirement. We could just as easily enable various gcc options and require other coding conventions. Furthermore we have other tools that are supposed to find things like this (e.g. coverity) but they are not being used. Sam