linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* RE: (0 == foo), rather than (foo == 0)
@ 2004-03-11  4:04 Godbole, Amarendra (GE Consumer & Industrial)
  0 siblings, 0 replies; 26+ messages in thread
From: Godbole, Amarendra (GE Consumer & Industrial) @ 2004-03-11  4:04 UTC (permalink / raw)
  To: linux-kernel

> As a result, using the former just tends to increase peoples 
> confusion by making code harder to read, which in turn tends 
> to increase the chance of bugs.

Kindly don't insult the kernel developers' with such statements. ;-)
They are smart enough to understand such constructs, which, I 
deduce from what someone said earlier in this thread. So no 
confusion  would prevail here...[take it in a lighter vein folks, I 
am kidding.]

> So don't do it. 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.

Agreed and understood. Thanks.

Sincere apologies for my previous ``non-text wrapped'' post. This
won't happen henceforth.

Cheers,
Amarendra

--
Picture Perfect Engineering / These opinions are mine.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: (0 == foo), rather than (foo == 0)
  2004-03-12  1:51   ` Chris Johns
@ 2004-03-12 12:21     ` Richard B. Johnson
  0 siblings, 0 replies; 26+ messages in thread
From: Richard B. Johnson @ 2004-03-12 12:21 UTC (permalink / raw)
  To: Chris Johns; +Cc: Chip Salzenberg, linux-kernel

On Thu, 11 Mar 2004, Chris Johns wrote:

> Just to throw a little more gas on this particular fire (or maybe
> some water, depending on your point of view), when I worked for a while
> at the Evil Empire, the "const == variable" comparison was mandatory in
> the group I worked in. Odd to look at initially, I'll admit, but it
> still caught
> several potential bugs. One other thing that was mandatory was to
> set to NULL any pointer to memory area that was freed immediately after
> freeing it. Again, a good idea, although not a coding style issue.
>
> Chris
>

Well I'll take it as being proof that forcing a particular style
doesn't help make bug-free code since it didn't help Microsoft.

I'll bet that Microsoft has more bugs per KLOC than any other
software, ever. It is a formidable challenge to make software
that will beat it in that area. You really need to try. You
need to be cunning enough to get it through the compile-stage.
It's an art.

Cheers,
Dick Johnson
Penguin : Linux version 2.4.24 on an i686 machine (797.90 BogoMips).
            Note 96.31% of all statistics are fiction.



^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: (0 == foo), rather than (foo == 0)
  2004-03-11 16:44 ` Chip Salzenberg
  2004-03-11 23:57   ` Peter Williams
@ 2004-03-12  1:51   ` Chris Johns
  2004-03-12 12:21     ` Richard B. Johnson
  1 sibling, 1 reply; 26+ messages in thread
From: Chris Johns @ 2004-03-12  1:51 UTC (permalink / raw)
  To: Chip Salzenberg; +Cc: linux-kernel

Just to throw a little more gas on this particular fire (or maybe
some water, depending on your point of view), when I worked for a while
at the Evil Empire, the "const == variable" comparison was mandatory in
the group I worked in. Odd to look at initially, I'll admit, but it 
still caught
several potential bugs. One other thing that was mandatory was to
set to NULL any pointer to memory area that was freed immediately after
freeing it. Again, a good idea, although not a coding style issue.

Chris


On Mar 11, 2004, at 10:44 AM, Chip Salzenberg wrote:

> Amarendra.Godbole@ge.com writes:
>>> As a result, using the former just tends to increase peoples
>>> confusion by making code harder to read, which in turn tends
>>> to increase the chance of bugs.
>>
>> Kindly don't insult the kernel developers' with such statements. ;-)
>> They are smart enough to understand such constructs [...]
>
> It's not about intelligence!  It's about the nature of human visual
> pattern-matching.  Reading a pattern is always easier when you've seen
> it thousands of times before.
>
> Henry Spencer's dictum about brace style seems particularly apropos:
>
> 8.  Thou shalt make thy program's purpose and structure clear to thy
>     fellow man by using the One True Brace Style, even if thou likest
>     it not, for thy creativity is better used in solving problems than
>     in creating beautiful new impediments to understanding.
>
> And that's what "0 == foo" is: an impediment to understanding.
> -- 
> Chip Salzenberg               - a.k.a. -               <chip@pobox.com>
> "I wanted to play hopscotch with the impenetrable mystery of existence,
>     but he stepped in a wormhole and had to go in early."  // MST3K
> -
> 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/
>


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: (0 == foo), rather than (foo == 0)
  2004-03-11 16:44 ` Chip Salzenberg
@ 2004-03-11 23:57   ` Peter Williams
  2004-03-12  1:51   ` Chris Johns
  1 sibling, 0 replies; 26+ messages in thread
From: Peter Williams @ 2004-03-11 23:57 UTC (permalink / raw)
  To: Chip Salzenberg; +Cc: linux-kernel

Chip Salzenberg wrote:
> Amarendra.Godbole@ge.com writes:
> 
>>>As a result, using the former just tends to increase peoples 
>>>confusion by making code harder to read, which in turn tends 
>>>to increase the chance of bugs.
>>
>>Kindly don't insult the kernel developers' with such statements. ;-)
>>They are smart enough to understand such constructs [...]
> 
> 
> It's not about intelligence!  It's about the nature of human visual
> pattern-matching.  Reading a pattern is always easier when you've seen
> it thousands of times before.
> 
> Henry Spencer's dictum about brace style seems particularly apropos:
> 
> 8.  Thou shalt make thy program's purpose and structure clear to thy
>     fellow man by using the One True Brace Style, even if thou likest
>     it not, for thy creativity is better used in solving problems than
>     in creating beautiful new impediments to understanding.
> 
> And that's what "0 == foo" is: an impediment to understanding.

It's got nothing to do with visual scanning.  It's more to do with 
grammatical form i.e. we (especially English speakers) like the form: 
subject (foo) verb (==) object (0); and in these cases the variable 
(foo) is the indisputable subject (i.e. the thing the sentence is about) 
but (0 == foo) momentarily causes us to think that 0 is the subject and 
we find this disconcerting.  On the other hand, Yoda (the Jedi master) 
would probably prefer: (0 foo ==) :-)

Like most matters of style there are points for and against both methods 
  and eventually someone has to make a (relatively arbitrary) decision. 
  Linus has made the decision and it's his call so we should abide by it 
(especially since he made it pretty clear that he was aware of (and 
considered) all the relevant arguments and is therefore unlikely to 
change his mind).

Peter
PS It's important not to get too emotionally attached to a particular 
method or style because there's always a chance that whoever gets to 
decide which will be used may not choose the one you like.  This time 
you lucked in :-)
-- 
Dr Peter Williams, Chief Scientist                peterw@aurema.com
Aurema Pty Limited                                Tel:+61 2 9698 2322
PO Box 305, Strawberry Hills NSW 2012, Australia  Fax:+61 2 9699 9174
79 Myrtle Street, Chippendale NSW 2008, Australia http://www.aurema.com


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: (0 == foo), rather than (foo == 0)
  2004-03-11 17:42                   ` Stefan Smietanowski
@ 2004-03-11 23:05                     ` Bill Davidsen
  0 siblings, 0 replies; 26+ messages in thread
From: Bill Davidsen @ 2004-03-11 23:05 UTC (permalink / raw)
  To: Stefan Smietanowski; +Cc: Andreas Schwab, Måns Rullgård, linux-kernel

Stefan Smietanowski wrote:

>> But this works:  while (a = b, a != 0).
>> (not that it is any better readable :-) ).
> 
> 
> My eyes! *Starts clawing them out*

All in what you are used to reading. I grew up with ranges sepcified as 
something like
   0 <= x <= 40
in the spec, and therefore still write it like
   if (0 <= x && x <= 40)
because that makes it obvious (to me) that it's a range check.

A case of the language not having a range check operator and various 
people finding one thing or another readable.

-- 
		-bill

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: (0 == foo), rather than (foo == 0)
  2004-03-11 15:18                 ` Andreas Schwab
@ 2004-03-11 17:42                   ` Stefan Smietanowski
  2004-03-11 23:05                     ` Bill Davidsen
  0 siblings, 1 reply; 26+ messages in thread
From: Stefan Smietanowski @ 2004-03-11 17:42 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: Måns Rullgård, linux-kernel

Hi Andreas.

>>>>The warning should be there whether there are parenthesis or not,
>>>>and it should state that you should have an explicit inequality
>>>>expression. So if you have
>>>>	if (a = b) 		...
>>>>and you really _mean_ that, then the way to write it sanely is to
>>>>just write it as
>>>>	if ((a = b) != 0)
>>>>		...
>>>>which makes it much clearer what you're actually doing.
>>>
>>>Or actually change it to
>>>
>>>a = b;
>>>if (a)
>>
>>That doesn't work with while().
> 
> 
> But this works:  while (a = b, a != 0).
> (not that it is any better readable :-) ).

My eyes! *Starts clawing them out*

// Stefan

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: (0 == foo), rather than (foo == 0)
       [not found] <1ypPV-5N2-3@gated-at.bofh.it>
@ 2004-03-11 16:44 ` Chip Salzenberg
  2004-03-11 23:57   ` Peter Williams
  2004-03-12  1:51   ` Chris Johns
  0 siblings, 2 replies; 26+ messages in thread
From: Chip Salzenberg @ 2004-03-11 16:44 UTC (permalink / raw)
  To: linux-kernel

Amarendra.Godbole@ge.com writes:
>> As a result, using the former just tends to increase peoples 
>> confusion by making code harder to read, which in turn tends 
>> to increase the chance of bugs.
>
>Kindly don't insult the kernel developers' with such statements. ;-)
>They are smart enough to understand such constructs [...]

It's not about intelligence!  It's about the nature of human visual
pattern-matching.  Reading a pattern is always easier when you've seen
it thousands of times before.

Henry Spencer's dictum about brace style seems particularly apropos:

8.  Thou shalt make thy program's purpose and structure clear to thy
    fellow man by using the One True Brace Style, even if thou likest
    it not, for thy creativity is better used in solving problems than
    in creating beautiful new impediments to understanding.

And that's what "0 == foo" is: an impediment to understanding.
-- 
Chip Salzenberg               - a.k.a. -               <chip@pobox.com>
"I wanted to play hopscotch with the impenetrable mystery of existence,
    but he stepped in a wormhole and had to go in early."  // MST3K

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: (0 == foo), rather than (foo == 0)
       [not found]         ` <1yALu-288-5@gated-at.bofh.it>
@ 2004-03-11 16:41           ` Chip Salzenberg
  0 siblings, 0 replies; 26+ messages in thread
From: Chip Salzenberg @ 2004-03-11 16:41 UTC (permalink / raw)
  To: linux-kernel

Valdis.Kletnieks@vt.edu writes:
>+       if ((options == (__WCLONE|__WALL)) && (current->uid = 0))

I remember that.  I wasn't scanning changes, but when it was shown up
in the news stories, I wasn't visually fooled for even a second.  You
can't code in C for decades without picking up good = vs. == radar.
-- 
Chip Salzenberg               - a.k.a. -               <chip@pobox.com>
"I wanted to play hopscotch with the impenetrable mystery of existence,
    but he stepped in a wormhole and had to go in early."  // MST3K

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: (0 == foo), rather than (foo == 0)
  2004-03-11 15:22       ` Richard B. Johnson
@ 2004-03-11 15:48         ` Valdis.Kletnieks
  0 siblings, 0 replies; 26+ messages in thread
From: Valdis.Kletnieks @ 2004-03-11 15:48 UTC (permalink / raw)
  To: root
  Cc: Randy.Dunlap, Godbole, Amarendra (GE Consumer & Industrial),
	linux-kernel

[-- Attachment #1: Type: text/plain, Size: 361 bytes --]

On Thu, 11 Mar 2004 10:22:49 EST, "Richard B. Johnson" said:

> No, and if you bothered to look at any code in the kernel, you'd
> note that uid and friends are always associated with some little
> pointy thingys. So, it wouldn't have been coded anything like that,
> anyway.

Fine. 

+       if ((options == (__WCLONE|__WALL)) && (current->uid = 0))

Happy???

[-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --]

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: (0 == foo), rather than (foo == 0)
  2004-03-11 15:03     ` Valdis.Kletnieks
@ 2004-03-11 15:22       ` Richard B. Johnson
  2004-03-11 15:48         ` Valdis.Kletnieks
  0 siblings, 1 reply; 26+ messages in thread
From: Richard B. Johnson @ 2004-03-11 15:22 UTC (permalink / raw)
  To: Valdis.Kletnieks
  Cc: Randy.Dunlap, Godbole, Amarendra (GE Consumer & Industrial),
	linux-kernel

On Thu, 11 Mar 2004 Valdis.Kletnieks@vt.edu wrote:

> On Wed, 10 Mar 2004 13:33:53 EST, "Richard B. Johnson" said:
>
> > People who develop kernel code know the difference between
> > '==' and '=' and are never confused my them. If you make
>
> Remember a few months ago when a lot of very clever people looked at
> a line of code that said 'if (yadda yadda && (uid = 0))' that had been
> inserted into the CVS tree, and it took a while for some of the very clever
> people to notice the equally clever hack?
>

No, and if you bothered to look at any code in the kernel, you'd
note that uid and friends are always associated with some little
pointy thingys. So, it wouldn't have been coded anything like that,
anyway.

Cheers,
Dick Johnson
Penguin : Linux version 2.4.24 on an i686 machine (797.90 BogoMips).
            Note 96.31% of all statistics are fiction.



^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: (0 == foo), rather than (foo == 0)
  2004-03-11  9:48               ` Måns Rullgård
  2004-03-11 10:29                 ` Stefan Smietanowski
@ 2004-03-11 15:18                 ` Andreas Schwab
  2004-03-11 17:42                   ` Stefan Smietanowski
  1 sibling, 1 reply; 26+ messages in thread
From: Andreas Schwab @ 2004-03-11 15:18 UTC (permalink / raw)
  To: Måns Rullgård; +Cc: linux-kernel

mru@kth.se (Måns Rullgård) writes:

> Stefan Smietanowski <stesmi@stesmi.com> writes:
>
>> Hi Linus.
>>
>>> The warning should be there whether there are parenthesis or not,
>>> and it should state that you should have an explicit inequality
>>> expression. So if you have
>>> 	if (a = b) 		...
>>> and you really _mean_ that, then the way to write it sanely is to
>>> just write it as
>>> 	if ((a = b) != 0)
>>> 		...
>>> which makes it much clearer what you're actually doing.
>>
>> Or actually change it to
>>
>> a = b;
>> if (a)
>
> That doesn't work with while().

But this works:  while (a = b, a != 0).
(not that it is any better readable :-) ).

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux AG, Maxfeldstraße 5, 90409 Nürnberg, Germany
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: (0 == foo), rather than (foo == 0)
  2004-03-10 18:33   ` Richard B. Johnson
  2004-03-10 23:00     ` Peter Williams
@ 2004-03-11 15:03     ` Valdis.Kletnieks
  2004-03-11 15:22       ` Richard B. Johnson
  1 sibling, 1 reply; 26+ messages in thread
From: Valdis.Kletnieks @ 2004-03-11 15:03 UTC (permalink / raw)
  To: root
  Cc: Randy.Dunlap, Godbole, Amarendra (GE Consumer & Industrial),
	linux-kernel

[-- Attachment #1: Type: text/plain, Size: 441 bytes --]

On Wed, 10 Mar 2004 13:33:53 EST, "Richard B. Johnson" said:

> People who develop kernel code know the difference between
> '==' and '=' and are never confused my them. If you make

Remember a few months ago when a lot of very clever people looked at
a line of code that said 'if (yadda yadda && (uid = 0))' that had been
inserted into the CVS tree, and it took a while for some of the very clever
people to notice the equally clever hack?

[-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --]

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: (0 == foo), rather than (foo == 0)
  2004-03-11  9:48               ` Måns Rullgård
@ 2004-03-11 10:29                 ` Stefan Smietanowski
  2004-03-11 15:18                 ` Andreas Schwab
  1 sibling, 0 replies; 26+ messages in thread
From: Stefan Smietanowski @ 2004-03-11 10:29 UTC (permalink / raw)
  To: Måns Rullgård; +Cc: linux-kernel

Hi Måns.

>>>The warning should be there whether there are parenthesis or not,
>>>and it should state that you should have an explicit inequality
>>>expression. So if you have
>>>	if (a = b) 		...
>>>and you really _mean_ that, then the way to write it sanely is to
>>>just write it as
>>>	if ((a = b) != 0)
>>>		...
>>>which makes it much clearer what you're actually doing.
>>
>>Or actually change it to
>>
>>a = b;
>>if (a)
> 
> That doesn't work with while().
> 

True. Didn't think of that but then the example used
was an if().

// Stefan

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: (0 == foo), rather than (foo == 0)
  2004-03-11  4:40             ` Stefan Smietanowski
@ 2004-03-11  9:48               ` Måns Rullgård
  2004-03-11 10:29                 ` Stefan Smietanowski
  2004-03-11 15:18                 ` Andreas Schwab
  0 siblings, 2 replies; 26+ messages in thread
From: Måns Rullgård @ 2004-03-11  9:48 UTC (permalink / raw)
  To: linux-kernel

Stefan Smietanowski <stesmi@stesmi.com> writes:

> Hi Linus.
>
>> The warning should be there whether there are parenthesis or not,
>> and it should state that you should have an explicit inequality
>> expression. So if you have
>> 	if (a = b) 		...
>> and you really _mean_ that, then the way to write it sanely is to
>> just write it as
>> 	if ((a = b) != 0)
>> 		...
>> which makes it much clearer what you're actually doing.
>
> Or actually change it to
>
> a = b;
> if (a)

That doesn't work with while().

-- 
Måns Rullgård
mru@kth.se


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: (0 == foo), rather than (foo == 0)
  2004-03-11  6:50         ` Willy Tarreau
@ 2004-03-11  7:36           ` Peter Williams
  0 siblings, 0 replies; 26+ messages in thread
From: Peter Williams @ 2004-03-11  7:36 UTC (permalink / raw)
  To: Willy Tarreau
  Cc: Linus Torvalds, root, linux-kernel, Godbole,
	Amarendra (GE Consumer & Industrial),
	Randy.Dunlap

Willy Tarreau wrote:
> Hi,
> 
> On Wed, Mar 10, 2004 at 06:36:22PM -0800, Linus Torvalds wrote:
> 
>>And while "0 == foo" may be logically the same thing as "foo == 0", the 
>>fact is, the latter is what people are used to seeing. And by being used 
>>to seeing it, they have an easier time thinking about it.
>>
>>As a result, using the former just tends to increase peoples confusion by
>>making code harder to read, which in turn tends to increase the chance of 
>>bugs.
> 
> 
> I have a friend who constantly uses it, and his code is unreadable, because
> sometimes, a "0 == xxx" becomes "0 <= xxx" or "0 >= xxx" which is difficult
> to understand. Thinking that xxx is negative because it's written on the
> right side of a >= is complicated. And the worst he does is when he uses
> functions : 
> 
>    if (0 < strcmp(a, "xxx")) ...
>    if (sizeof(t) > read(fd, t, sizeof(t)) ...
> 
> I have already helped him track bugs in his programs, and some of them were
> just related to this usage, because nobody's brain can understand these
> constructions immediately without thinking a bit. So I'm all against this
> sort of thing.

One final note.  I agree with all the statements of how awkward and 
unnatural the back to front boolean expressions look but I had adopted 
this technique (for myself) as a means of overcoming design shortcomings 
in the C language.  I intend to keep doing it in my private code (as 
it's saved my bacon a number of times) but will conform to Linus's 
standards for any contributions/patches I submit for kernel code (just 
as I would conform for any other person's standards if I were to 
contribute to their work).  In the long run, consistency in a body of 
code greatly enhances its readability.

Peace?
Peter
-- 
Dr Peter Williams, Chief Scientist                peterw@aurema.com
Aurema Pty Limited                                Tel:+61 2 9698 2322
PO Box 305, Strawberry Hills NSW 2012, Australia  Fax:+61 2 9699 9174
79 Myrtle Street, Chippendale NSW 2008, Australia http://www.aurema.com


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: (0 == foo), rather than (foo == 0)
  2004-03-11  2:36       ` Linus Torvalds
  2004-03-11  3:08         ` Peter Williams
@ 2004-03-11  6:50         ` Willy Tarreau
  2004-03-11  7:36           ` Peter Williams
  1 sibling, 1 reply; 26+ messages in thread
From: Willy Tarreau @ 2004-03-11  6:50 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Peter Williams, root, Randy.Dunlap, linux-kernel, Godbole,
	Amarendra (GE Consumer & Industrial)

Hi,

On Wed, Mar 10, 2004 at 06:36:22PM -0800, Linus Torvalds wrote:
> 
> And while "0 == foo" may be logically the same thing as "foo == 0", the 
> fact is, the latter is what people are used to seeing. And by being used 
> to seeing it, they have an easier time thinking about it.
> 
> As a result, using the former just tends to increase peoples confusion by
> making code harder to read, which in turn tends to increase the chance of 
> bugs.

I have a friend who constantly uses it, and his code is unreadable, because
sometimes, a "0 == xxx" becomes "0 <= xxx" or "0 >= xxx" which is difficult
to understand. Thinking that xxx is negative because it's written on the
right side of a >= is complicated. And the worst he does is when he uses
functions : 

   if (0 < strcmp(a, "xxx")) ...
   if (sizeof(t) > read(fd, t, sizeof(t)) ...

I have already helped him track bugs in his programs, and some of them were
just related to this usage, because nobody's brain can understand these
constructions immediately without thinking a bit. So I'm all against this
sort of thing.

Cheers,
Willy


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: (0 == foo), rather than (foo == 0)
  2004-03-11  3:19           ` Linus Torvalds
@ 2004-03-11  4:40             ` Stefan Smietanowski
  2004-03-11  9:48               ` Måns Rullgård
  0 siblings, 1 reply; 26+ messages in thread
From: Stefan Smietanowski @ 2004-03-11  4:40 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Peter Williams, Randy.Dunlap, root, linux-kernel, Godbole,
	Amarendra (GE Consumer & Industrial)

Hi Linus.

> The warning should be there whether there are parenthesis or not, and it 
> should state that you should have an explicit inequality expression. So if 
> you have
> 
> 	if (a = b) 
> 		...
> 
> and you really _mean_ that, then the way to write it sanely is to just 
> write it as
> 
> 	if ((a = b) != 0)
> 		...
> 
> which makes it much clearer what you're actually doing.

Or actually change it to

a = b;
if (a)
   ...

That's even more clear in my opinion. The other still feels a bit iffy
but maybe that's just me.

// Stefan

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: (0 == foo), rather than (foo == 0)
  2004-03-11  3:08         ` Peter Williams
@ 2004-03-11  3:19           ` Linus Torvalds
  2004-03-11  4:40             ` Stefan Smietanowski
  0 siblings, 1 reply; 26+ messages in thread
From: Linus Torvalds @ 2004-03-11  3:19 UTC (permalink / raw)
  To: Peter Williams
  Cc: Randy.Dunlap, root, linux-kernel, Godbole,
	Amarendra (GE Consumer & Industrial)



On Thu, 11 Mar 2004, Peter Williams wrote:
> 
> As somebody pointed out -Wall will (help) detect most of these errors by 
> suggesting () be placed around any expression of the form a = b that 
> occurs inside a simple boolean expression which will cause those people 
> who care about eliminating warning messages to reevaluate the code and 
> make sure they really meant a = b and replace it with (a = b) to get rid 
> of the warning error.

Actually, don't just add parenthesis. They get rid of the warning, but 
they don't actually make for very pretty reading.

I don't know why the gcc warning suggests adding parentheses, since they 
have no semantic meaning, and are _horrible_ from a syntactic standpoint.

The warning should be there whether there are parenthesis or not, and it 
should state that you should have an explicit inequality expression. So if 
you have

	if (a = b) 
		...

and you really _mean_ that, then the way to write it sanely is to just 
write it as

	if ((a = b) != 0)
		...

which makes it much clearer what you're actually doing.

		Linus

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: (0 == foo), rather than (foo == 0)
  2004-03-11  2:36       ` Linus Torvalds
@ 2004-03-11  3:08         ` Peter Williams
  2004-03-11  3:19           ` Linus Torvalds
  2004-03-11  6:50         ` Willy Tarreau
  1 sibling, 1 reply; 26+ messages in thread
From: Peter Williams @ 2004-03-11  3:08 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Randy.Dunlap, root, linux-kernel, Godbole,
	Amarendra (GE Consumer & Industrial)

Linus Torvalds wrote:
> 
> On Thu, 11 Mar 2004, Peter Williams wrote:
> 
>>Richard B. Johnson wrote:
>>
>>>People who develop kernel code know the difference between
>>>'==' and '=' and are never confused my them.
>>
>>And you never make typing mistakes?  That's admirable or should I say 
>>incredible.
> 
> 
> The thing is, people tend to make fewer typing mistakes of that kind, than 
> just plain logic errors from not thinking right about something.
> 
> And while "0 == foo" may be logically the same thing as "foo == 0", the 
> fact is, the latter is what people are used to seeing. And by being used 
> to seeing it, they have an easier time thinking about it.
> 
> As a result, using the former just tends to increase peoples confusion by
> making code harder to read, which in turn tends to increase the chance of 
> bugs.
> 
> So don't do it. 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.
> 
> 		Linus

OK.  I'll change all of such occurences in our EBS patches.

As somebody pointed out -Wall will (help) detect most of these errors by 
suggesting () be placed around any expression of the form a = b that 
occurs inside a simple boolean expression which will cause those people 
who care about eliminating warning messages to reevaluate the code and 
make sure they really meant a = b and replace it with (a = b) to get rid 
of the warning error.

The reason that I say "most" rather than "all" is (that testing shows) 
that if the a = b is part of a more complex expression and is already in 
() in order to (for instance) ensure the desired precedence -Wall will 
not flag it as a possible problem.

Peter
-- 
Dr Peter Williams, Chief Scientist                peterw@aurema.com
Aurema Pty Limited                                Tel:+61 2 9698 2322
PO Box 305, Strawberry Hills NSW 2012, Australia  Fax:+61 2 9699 9174
79 Myrtle Street, Chippendale NSW 2008, Australia http://www.aurema.com



^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: (0 == foo), rather than (foo == 0)
  2004-03-10 23:00     ` Peter Williams
  2004-03-11  0:16       ` Randy.Dunlap
@ 2004-03-11  2:36       ` Linus Torvalds
  2004-03-11  3:08         ` Peter Williams
  2004-03-11  6:50         ` Willy Tarreau
  1 sibling, 2 replies; 26+ messages in thread
From: Linus Torvalds @ 2004-03-11  2:36 UTC (permalink / raw)
  To: Peter Williams
  Cc: root, Randy.Dunlap, linux-kernel, Godbole,
	Amarendra (GE Consumer & Industrial)



On Thu, 11 Mar 2004, Peter Williams wrote:
> Richard B. Johnson wrote:
> > 
> > People who develop kernel code know the difference between
> > '==' and '=' and are never confused my them.
> 
> And you never make typing mistakes?  That's admirable or should I say 
> incredible.

The thing is, people tend to make fewer typing mistakes of that kind, than 
just plain logic errors from not thinking right about something.

And while "0 == foo" may be logically the same thing as "foo == 0", the 
fact is, the latter is what people are used to seeing. And by being used 
to seeing it, they have an easier time thinking about it.

As a result, using the former just tends to increase peoples confusion by
making code harder to read, which in turn tends to increase the chance of 
bugs.

So don't do it. 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.

		Linus

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: (0 == foo), rather than (foo == 0)
  2004-03-10 23:00     ` Peter Williams
@ 2004-03-11  0:16       ` Randy.Dunlap
  2004-03-11  2:36       ` Linus Torvalds
  1 sibling, 0 replies; 26+ messages in thread
From: Randy.Dunlap @ 2004-03-11  0:16 UTC (permalink / raw)
  To: Peter Williams; +Cc: root, linux-kernel, Amarendra.Godbole

On Thu, 11 Mar 2004 10:00:56 +1100 Peter Williams wrote:

| Richard B. Johnson wrote:
| > On Wed, 10 Mar 2004, Randy.Dunlap wrote:
| > 
| > 
| >>On Wed, 10 Mar 2004 11:46:40 +0530 Godbole, Amarendra \(GE Consumer &
| >>Industrial\) wrote:
| >>
| >>Hi,
| >>
| >>While writing code, the assignment operator (=) is many-a-times
| >>confused with the comparison operator (==) resulting in very subtle
| >>bugs difficult to track. To keep a check on this -- the constant
| >>can be written on the LHS rather than the RHS which will result
| >>in a compile time error if wrong operator is used.
| >>
| > 
| > 
| > People who develop kernel code know the difference between
| > '==' and '=' and are never confused my them.
| 
| And you never make typing mistakes?  That's admirable or should I say 
| incredible.

Whoa.  stop the presses.  an on-$subejct posting.

Please, let's kill all the other fuss about email crapola...

--
~Randy

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: (0 == foo), rather than (foo == 0)
  2004-03-10 18:33   ` Richard B. Johnson
@ 2004-03-10 23:00     ` Peter Williams
  2004-03-11  0:16       ` Randy.Dunlap
  2004-03-11  2:36       ` Linus Torvalds
  2004-03-11 15:03     ` Valdis.Kletnieks
  1 sibling, 2 replies; 26+ messages in thread
From: Peter Williams @ 2004-03-10 23:00 UTC (permalink / raw)
  To: root
  Cc: Randy.Dunlap, linux-kernel, Godbole,
	Amarendra (GE Consumer & Industrial)

Richard B. Johnson wrote:
> On Wed, 10 Mar 2004, Randy.Dunlap wrote:
> 
> 
>>On Wed, 10 Mar 2004 11:46:40 +0530 Godbole, Amarendra \(GE Consumer &
>>Industrial\) wrote:
>>
>>Hi,
>>
>>While writing code, the assignment operator (=) is many-a-times
>>confused with the comparison operator (==) resulting in very subtle
>>bugs difficult to track. To keep a check on this -- the constant
>>can be written on the LHS rather than the RHS which will result
>>in a compile time error if wrong operator is used.
>>
> 
> 
> People who develop kernel code know the difference between
> '==' and '=' and are never confused my them.

And you never make typing mistakes?  That's admirable or should I say 
incredible.

> If you make
> contributions to kernel code, and write: "if (0==foo)", your
> code will be reviewed until it is obsolete and never find
> its way into the kernel. Please don't insult kernel developers
> with this kind of kid-stuff.
> 
> People who develop kernel code also know what a line-warp is.
> They put a '\n' "[Enter] key" in their text every so-often,
> maybe every 70 to 79 characters...
> 
> Cheers,
> Dick Johnson
> Penguin : Linux version 2.4.24 on an i686 machine (797.90 BogoMips).
>             Note 96.31% of all statistics are fiction.
> 
> 
> -
> 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/


-- 
Dr Peter Williams, Chief Scientist                peterw@aurema.com
Aurema Pty Limited                                Tel:+61 2 9698 2322
PO Box 305, Strawberry Hills NSW 2012, Australia  Fax:+61 2 9699 9174
79 Myrtle Street, Chippendale NSW 2008, Australia http://www.aurema.com


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: (0 == foo), rather than (foo == 0)
  2004-03-10 18:02 ` Randy.Dunlap
@ 2004-03-10 18:33   ` Richard B. Johnson
  2004-03-10 23:00     ` Peter Williams
  2004-03-11 15:03     ` Valdis.Kletnieks
  0 siblings, 2 replies; 26+ messages in thread
From: Richard B. Johnson @ 2004-03-10 18:33 UTC (permalink / raw)
  To: Randy.Dunlap
  Cc: Godbole, Amarendra (GE Consumer & Industrial), linux-kernel

On Wed, 10 Mar 2004, Randy.Dunlap wrote:

> On Wed, 10 Mar 2004 11:46:40 +0530 Godbole, Amarendra \(GE Consumer &
> Industrial\) wrote:
>
> Hi,
>
> While writing code, the assignment operator (=) is many-a-times
> confused with the comparison operator (==) resulting in very subtle
> bugs difficult to track. To keep a check on this -- the constant
> can be written on the LHS rather than the RHS which will result
> in a compile time error if wrong operator is used.
>

People who develop kernel code know the difference between
'==' and '=' and are never confused my them. If you make
contributions to kernel code, and write: "if (0==foo)", your
code will be reviewed until it is obsolete and never find
its way into the kernel. Please don't insult kernel developers
with this kind of kid-stuff.

People who develop kernel code also know what a line-warp is.
They put a '\n' "[Enter] key" in their text every so-often,
maybe every 70 to 79 characters...

Cheers,
Dick Johnson
Penguin : Linux version 2.4.24 on an i686 machine (797.90 BogoMips).
            Note 96.31% of all statistics are fiction.



^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: (0 == foo), rather than (foo == 0)
  2004-03-10  6:16 Godbole, Amarendra (GE Consumer & Industrial)
  2004-03-10 10:34 ` Bernd Petrovitsch
@ 2004-03-10 18:02 ` Randy.Dunlap
  2004-03-10 18:33   ` Richard B. Johnson
  1 sibling, 1 reply; 26+ messages in thread
From: Randy.Dunlap @ 2004-03-10 18:02 UTC (permalink / raw)
  To: Godbole, Amarendra (GE Consumer & Industrial); +Cc: linux-kernel

On Wed, 10 Mar 2004 11:46:40 +0530 Godbole, Amarendra \(GE Consumer & Industrial\) wrote:

| Hi,
| 
| While writing code, the assignment operator (=) is many-a-times confused with the comparison operator (==) resulting in very subtle bugs difficult to track. To keep a check on this -- the constant can be written on the LHS rather than the RHS which will result in a compile time error if wrong operator is used.

Yes, we know.

| Is this an encouraged practice while writing any code for the kernel ? Or is this choice left to the developer ? I was unable to find any reference to it in the CodingStyle document. I did find some code under drivers/ which used (NULL == foo) and similar constructs. 

I prefer that it be left to each developer.

| Can this be added to the CodingStyle document ?

Hopefully not.  Some of us think that it's ugly.

BTW, can you use CR/LF every 70 characters or so, please?

--
~Randy

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: (0 == foo), rather than (foo == 0)
  2004-03-10  6:16 Godbole, Amarendra (GE Consumer & Industrial)
@ 2004-03-10 10:34 ` Bernd Petrovitsch
  2004-03-10 18:02 ` Randy.Dunlap
  1 sibling, 0 replies; 26+ messages in thread
From: Bernd Petrovitsch @ 2004-03-10 10:34 UTC (permalink / raw)
  To: Godbole, Amarendra (GE Consumer & Industrial); +Cc: linux-kernel

Reformatted long lines - please fix your mailer.

On Mit, 2004-03-10 at 07:16, Godbole, Amarendra (GE Consumer &
Industrial) wrote:
> While writing code, the assignment operator (=) is many-a-times confused
> with the comparison operator (==) resulting in very subtle bugs difficult
> to track. To keep a check on this -- the constant can be written on the 
> LHS rather than the RHS which will result in a compile time error if wrong
> operator is used.

Or you use `gcc -Wall` which also reports this (and also in the cases
where both sides of the comparison can be left hand sides).

	Bernd
-- 
Firmix Software GmbH                   http://www.firmix.at/
mobil: +43 664 4416156                 fax: +43 1 7890849-55
          Embedded Linux Development and Services


^ permalink raw reply	[flat|nested] 26+ messages in thread

* (0 == foo), rather than (foo == 0)
@ 2004-03-10  6:16 Godbole, Amarendra (GE Consumer & Industrial)
  2004-03-10 10:34 ` Bernd Petrovitsch
  2004-03-10 18:02 ` Randy.Dunlap
  0 siblings, 2 replies; 26+ messages in thread
From: Godbole, Amarendra (GE Consumer & Industrial) @ 2004-03-10  6:16 UTC (permalink / raw)
  To: linux-kernel

Hi,

While writing code, the assignment operator (=) is many-a-times confused with the comparison operator (==) resulting in very subtle bugs difficult to track. To keep a check on this -- the constant can be written on the LHS rather than the RHS which will result in a compile time error if wrong operator is used.

Is this an encouraged practice while writing any code for the kernel ? Or is this choice left to the developer ? I was unable to find any reference to it in the CodingStyle document. I did find some code under drivers/ which used (NULL == foo) and similar constructs. 

Can this be added to the CodingStyle document ?

Cheers,
Amarendra

--
#include <std$disclaimer.h>

^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2004-03-12 12:19 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2004-03-11  4:04 (0 == foo), rather than (foo == 0) Godbole, Amarendra (GE Consumer & Industrial)
     [not found] <1ypPV-5N2-3@gated-at.bofh.it>
2004-03-11 16:44 ` Chip Salzenberg
2004-03-11 23:57   ` Peter Williams
2004-03-12  1:51   ` Chris Johns
2004-03-12 12:21     ` Richard B. Johnson
     [not found] <1y5oc-8cr-1@gated-at.bofh.it>
     [not found] ` <1ygjH-3LE-31@gated-at.bofh.it>
     [not found]   ` <1ygMH-4eu-21@gated-at.bofh.it>
     [not found]     ` <1yzZ9-1qq-43@gated-at.bofh.it>
     [not found]       ` <1yAs0-1P6-7@gated-at.bofh.it>
     [not found]         ` <1yALu-288-5@gated-at.bofh.it>
2004-03-11 16:41           ` Chip Salzenberg
  -- strict thread matches above, loose matches on Subject: below --
2004-03-10  6:16 Godbole, Amarendra (GE Consumer & Industrial)
2004-03-10 10:34 ` Bernd Petrovitsch
2004-03-10 18:02 ` Randy.Dunlap
2004-03-10 18:33   ` Richard B. Johnson
2004-03-10 23:00     ` Peter Williams
2004-03-11  0:16       ` Randy.Dunlap
2004-03-11  2:36       ` Linus Torvalds
2004-03-11  3:08         ` Peter Williams
2004-03-11  3:19           ` Linus Torvalds
2004-03-11  4:40             ` Stefan Smietanowski
2004-03-11  9:48               ` Måns Rullgård
2004-03-11 10:29                 ` Stefan Smietanowski
2004-03-11 15:18                 ` Andreas Schwab
2004-03-11 17:42                   ` Stefan Smietanowski
2004-03-11 23:05                     ` Bill Davidsen
2004-03-11  6:50         ` Willy Tarreau
2004-03-11  7:36           ` Peter Williams
2004-03-11 15:03     ` Valdis.Kletnieks
2004-03-11 15:22       ` Richard B. Johnson
2004-03-11 15:48         ` Valdis.Kletnieks

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).