Wireshark · Wireshark-dev: Re: [Wireshark-dev] Yoda style conditions

[Jeff Morriss <jeff.morriss.ws@xxxxxxxxx>]

Steffen DETTMER wrote:
> (OT, sorry for my noise on the list)

Well, it IS a development discussion, so I'm not sure it's completely OT.

> Also, I think for NULL pointers this is much more easy than to
> check for possibly deleted pointers (or otherwise stale pointers
> or copies of pointers realloc'd after the copy etc).
> Unfortunately I have no clue how much a good static code analyzer
> can help here. Maybe it could spot some of such problems and
> assist developing, but if I had to guess I would guess
> determination of such conditions would be quite limited.
>
> Or is it considered good to add such if(p)-checks because they
> are cheap and could avoid a crash in at least some cases?

I usually think of it this way:

- If I think the pointer can logically be NULL (e.g., it'll be NULL if 
if it's possible we didn't hit the condition that initializes it to 
something else), then I put an if(p) check.

- else, just dereference the thing and crash if it's NULL/bogus/already 
freed[1]/uninitialized[1] so that there is (on "proper" ;-)) systems a 
core file to analyze.

I'm not a believer in always checking for things that can't be the case 
because, frankly, *when* they do happen, I'd much rather have a core 
file than some really, really obscure (and probably not reproducible) 
behavior.  (Of course this belief depends on living in an environment 
where I can get usable core files from crashes.)

[1] Memory scrubbers that overwrite the contents of memory as soon as it 
is freed and initialize it to non-zero when it is allocated help detect 
these kinds of problems during development/testing.  Of course so do 
things like glibc's malloc'd memory checking functions.