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

[Garrett D'Amore]

On Wed, 2011-04-20 at 16:25 -0400, Gordon Ross wrote:
> 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.

Agreed.

	- Garrett

> 
> Gordon
> 
> _______________________________________________
> Developer mailing list
> Developer at lists.illumos.org
> http://lists.illumos.org/m/listinfo/developer