From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752324AbcDOPum (ORCPT ); Fri, 15 Apr 2016 11:50:42 -0400 Received: from smtprelay0190.hostedemail.com ([216.40.44.190]:57696 "EHLO smtprelay.hostedemail.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751325AbcDOPuj (ORCPT ); Fri, 15 Apr 2016 11:50:39 -0400 X-Session-Marker: 6A6F6540706572636865732E636F6D X-Spam-Summary: 13,1.2,0,,d41d8cd98f00b204,joe@perches.com,:::::::::::::,RULES_HIT:41:355:379:541:599:800:960:973:982:988:989:1260:1277:1311:1313:1314:1345:1359:1373:1437:1515:1516:1518:1535:1544:1593:1594:1605:1711:1730:1747:1777:1792:2393:2553:2559:2562:2689:2693:2734:2736:2828:3138:3139:3140:3141:3142:3622:3865:3866:3867:3868:3870:3871:3872:3873:3874:4250:4321:5007:6691:7208:7809:7875:7903:8527:10008:10226:10848:11026:11232:11473:11658:11783:11914:12043:12114:12517:12519:12555:12740:13137:13150:13230:13231:13439:13894:14039:14093:14097:14181:14659:14721:21067:21080:21326:30054:30064:30065:30090:30091,0,RBL:none,CacheIP:none,Bayesian:0.5,0.5,0.5,Netcheck:none,DomainCache:0,MSF:not bulk,SPF:fn,MSBL:0,DNSBL:none,Custom_rules:0:1:0,LFtime:3,LUA_SUMMARY:none X-HE-Tag: train52_3f89368555f43 X-Filterd-Recvd-Size: 5082 Message-ID: <1460735428.19090.23.camel@perches.com> Subject: Re: [PATCH] drm/amdgpu: fix compare_const_fl.cocci warnings From: Joe Perches To: Julia Lawall , Christian =?ISO-8859-1?Q?K=F6nig?= Cc: Dave Airlie , kbuild-all@01.org, Alex Deucher , dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org Date: Fri, 15 Apr 2016 08:50:28 -0700 In-Reply-To: References: <5710AA53.7070307@amd.com> Content-Type: text/plain; charset="ISO-8859-1" X-Mailer: Evolution 3.18.5.2-0ubuntu1 Mime-Version: 1.0 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, 2016-04-15 at 16:20 +0200, Julia Lawall wrote: > On Fri, 15 Apr 2016, Christian König wrote: > > Am 15.04.2016 um 09:15 schrieb Julia Lawall: > > > Move constants to the right of binary operators. > > > > > > Generated by: scripts/coccinelle/misc/compare_const_fl.cocci > > > > > > Signed-off-by: Fengguang Wu > > > Signed-off-by: Julia Lawall > > > > In general the patch looks ok, but do we have a documented preference where to > > place constants in the coding style docs? > > > > While it's not so much of a problem any more with modern compilers, some > > people still prefer to have it on the left side to catch accidental value > > assignments. > > I don't know if it is documented.  Joe Perches suggested that on the right > was better in general - maybe he knows if this is written somewhere. > > There are 504 occurrences of NULL == in the kernel, and 19524 occurrences > of == NULL. A long time ago Linus wrote: > On Wed, 2004-03-10 at 18:36, Linus Torvalds wrote: > > > The kind of bug that the "0 == x" syntax protects against > > > is LESS LIKELY to happen than the kind of bug it tends to cause. > >  > > Not my experience.  I'd personally prefer a stated preference for. > >  > > (lvalue == rvalue) vs (rvalue == lvalue) >  > The thing is, your "vs" above makes no sense. >  > Quite often, both sides are rvalues, or lvalues. In fact, often you may > not even know from a simple syntactic look which one either side is, since  > they can be macros etc. >  > So asking for consistency is just impossible, because the exact same  > expression may semantically parse as either depending on things like  > macros that have architectural dependencies.  >  > So the rule would have to be something like this: >  - if one side is _obviously_ a lvalue, and the other side is _obviously_  >    a rvalue, then do X. >  > That kind of rule makes very little sense, but if you want a stated > preference, then my preference is to say that in that obvious case, the > lvalue comes on the left side, and the rvalue comes on the right side. >  > Why? Because that is literally how people think. People have been taught  > since before first grade to think "if I have 8 apples, and I give Tom one  > apple, how many apples do I have". >  > Notice how I didn't say "if 8 applies is what I have.." >  > The reason for "if (x == 8)" comes from the way we're taught to think.  > Arguing against that _fact_ is just totally non-productive, and you have  > to _force_ yourself to write it the other way around. >  > And that just means that you will do other mistakes. You'll spend your  > time thinking about trying to express your conditionals in strange ways,  > and then not think about the _real_ issue. >  > So let's make a few rules: >  >  - write your logical expressions the way people EXPECT them to be  >    written. No silly rules that make no sense. >  >    Ergo: >  >         if (x == 8) >  >    is the ONE AND ONLY SANE WAY. >  >  - avoid using assignment inside logical expressions unless you have a  >    damn good reason to. >  >    Ergo: write >  >         error = myfunction(xxxx) >         if (error) { >                 ... >  >    instead of writing >  >         if (error = myfunction(xxxx)) >                 .... >  >    which is just unreadable and stupid. >  >  - Don't get hung about stupid rules.  >  >    Ergo: sometimes assignments in conditionals make sense, especially in >    loops. Don't avoid them just because of some silly rule. But strive to >    use an explicit equality test when you do so: >  >         while ((a = function(b)) != 0)  >                 ... >  >    is fine. >  >  - The compiler warns about the mistakes that remain, if you follow these  >    rules. >  >  - mistakes happen. Deal with it. Having tons of rules just makes them  >    more likely. Expect mistakes, and make sure they are fixed quickly. >  > Is that "stated preference" enough? >