Wireshark · Wireshark-dev: [Wireshark-dev] Yoda style conditions (was: Re: [Wireshark-commits] rev 35975: /trunk/epan/dfilter/

[Steffen DETTMER <Steffen.DETTMER@xxxxxxxxxxxx>]

(OT)

* Jeff Morriss wrote on Thu, Feb 17, 2011 at 09:27 -0500:
> And some people think
> 
> 	if (NULL == a) {
> 
> is a good way of coding since a typo like (NULL = a) wouldn't compile.
> But (at least for me) that really is unreadable!  (And finding typos 
> like the above is, in my mind, something for the compiler to warn about.)

Yes, I agree.

I think such `Yoda style conditions' (`if blue is the sky')
nowadays are less important, because as long as you compile with
at least one compiler that gives a warning here, you'd be safe :)

$ echo "void f(int a) { if (a = 0) { } }" > x.c
$ gcc -Wall -c x.c
x.c: In function `f':
x.c:1: warning: suggest parentheses around assignment used as truth value
$ gcc --version
2.95.2

which is more than ten years old :)

> I imagine there are going to be a LOT of "if (a)" checks in 2 million 
> LOC.  Do we really want to change them all?

shouldn't the number of such problems reported by the static code
analyzer match the number of places to be changed?

(personally, I think in general it can be quite doubtful to add
some if (p != NULL) around *p, maybe p was checked before or was
initilized with some p = &o or whatever. I would be afraid that
such warning make some people to blindly add if (p != NULL) with
the possible result to make the app misbehave in a way hard to
find instead of crashing quick, straight-forward, direct and
clean, which is easy to debug. We even have a developer rule
usually NOT to use things like `if (p)' or `if (p != NULL)' [for
mandatory pointers, but assert(p) should be used] because
pointers cannot be checked; *p also crashes when already freed or
uninitialized or whatever - comments appreciated :)).

oki,

Steffen