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

[Michael Schuster]

On Wed, Apr 20, 2011 at 20:49, Dan McDonald <danmcd at nexenta.com> wrote:
> 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.

I don't know about a rule, but I do remember a discussion about this
very issue a few years ago (can't remember whether inside Sun only),
where there was a strong opinion about not treating pointers like
booleans (ie, agreeing with your POV, if I read it correctly) - meem
was the person putting it forward at the time, and FWIW I also prefer
keeping booleans and others apart in this way.

cheers
Michael
-- 
regards/mit freundlichen Grüssen
Michael Schuster