[illumos-Advocates] Fwd: [ZFS-WG] ZFS code review: TIMING OUT!

[Garrett D'Amore]

Ok, but go and mention Mark Maybee. :-)  List me as approver so you
don't catch any shit later -- the ZFS WG already knows this was
reasonably reviewed. :-)

	- Garrett

On Fri, 2011-05-13 at 23:56 -0400, Gordon Ross wrote:
> Lisa Week offered advice to basically, also look at the NFSv4 ACL
> code (and presumably try to sync. them up)  but I don't think we
> should do that.  ACLs are already different in the two.
> 
> Re: the comment about line 1578:
>          (void) isdir;   /* will need this later */
> 
> Yes, one could instead add before the function:
>         /* ARGSUSED */
> or, better still:
>         NOTE(ARGUNUSED(isdir))
> but that line is temporary, until the reworked fix for
> 807 (Trivial ACEs missing delete) comes back, so
> I don't think it's worth the trouble to change now.
> .
> In short, nothing new here.  Let's push it.
> 
> Everyone has had ample opportunity to both
> look at the code (if they want) and to comment
> and/or get their name on it as a reviewer (if
> they want to).
> 
> I suggest we not mention Mark S. or Lisa W.
> among reviewers.  It's enough that we know
> they looked and had no major concerns.
> 
> Gordon
> 
> On Fri, May 13, 2011 at 11:30 PM, Garrett D'Amore <garrett at nexenta.com> wrote:
> >
> >
> >   -- Garrett D'Amore
> > Begin forwarded message:
> [...]
> > Subject: Re: Fwd: Fwd: [ZFS-WG] ZFS code review: TIMING OUT!
> > Date: Tue, 10 May 2011 15:47:51 -0600
> > From: Lisa Week <lisa.week at oracle.com>
> > To: Mark Shellenbaum <mark.shellenbaum at oracle.com>
> >
> > Here's the feedback that I had.
> >
> > acl_common.c
> > General comment - Many of the changes made here (e.g. changing isdir to
> > boolean_t types everywhere) will make the code diverge from the original
> > place that it came from (the NFSv4 ACL translators in nfs4_acl.c).  This
> > isn't really a show stopper, but divergence from the original is
> > something you should be aware of.  (Of course, you the best solution
> > would be to the move the common functions into into usr/src/common or
> > whatever.)
> >
> > Line 1578: Should be removed.  Why not just use /*ARGSUSED*/ to get rid
> > of lint warnings?
> >
> > - Lisa
> >
> 
> _______________________________________________
> Advocates mailing list
> Advocates at lists.illumos.org
> http://lists.illumos.org/m/listinfo/advocates