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

[Gordon Ross]

On Wed, Apr 20, 2011 at 2:49 PM, 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.
>
> Thanks,
> Dan

I generally do this (and other style stuff) only in cases where I'm making
other, more significant changes to the file.  Don't think I'd bother here.

Gordon