[illumos-Developer] CODE REVIEW --> #935, sv_lyr_open() uses NULL pointers...

Dan McDonald danmcd at nexenta.com
Wed Apr 20 11:49:11 PDT 2011


On Wed, Apr 20, 2011 at 10:05:57AM -0700, Adam Leventhal wrote:
> Hey Dan,
> 
> You've changed some boolean uses of svp to be svp != NULL, but there
> are several other places in the function that you didn't change. I'd
> argue for consistency one way or the other (and probably within the
> file).

Yeah...  I got a unicast reply suggesting that I just do the one fix and be
done with it.  I personally find:

	ptr_t *blah;

	...

	if (!blah)  /* XXX KEBE ASKS Why treat like a boolean? */

to make code somewhat less readable.  What I can't remember is if we had an
official ON/Illumos Big Rule (TM) about this.  The code I'm fixing was
inherited from the old Network Storage (NWS) consolidation, which makes me
wonder even more.

> But the code itself looks good.

The one null-pointer check is the big fix.  I'm deciding whether or not to go
all-in and fix the file to be readable or minimize the changes to be just
this line:

+	if (svp != NULL)
|		rw_exit(&svp->....);

I'll pick one and run with it on my own, but I would be interested in knowing
if there is an unwritten ON/Illumos Big Rule (TM) about this.

Thanks,
Dan



More information about the Developer mailing list