[PATCH] drm/amdgpu: fix compare_const_fl.cocci warnings
Joe Perches
joe at perches.com
Fri Apr 15 15:50:28 UTC 2016
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 <fengguang.wu at intel.com>
> > > Signed-off-by: Julia Lawall <julia.lawall at lip6.fr>
> >
> > 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?
>
More information about the dri-devel
mailing list