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

[Gordon Ross]

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
>