[illumos-Developer] [RFC] Dead code in smbfs client

[Gordon Ross]

That was a feature of both the BSD/Apple code this was ported from,
and of the current NFS client code (rnode.h struct rnode r_cred).
We left it there because we were not sure if we would need the
actual credentials used during the open call.  While nothing in
smbfs appears to need r_cred today, that could change...
(i.e. if/when someone adds mmap support)

In short, I'm not too worried about this r_cred.

Gordon

On Wed, May 18, 2011 at 12:16 AM, Dan Kruchinin <dkruchinin at acm.org> wrote:
> Hi list.
>
> Pleas look at uts/common/fs/smbclnt/smbfs_node.h
> It contains definition of smbnode structure that contains a field:
>        cred_t          *r_cred;        /* current credentials */
>
> This filed is accessed from four functions
> File uts/common/fs/smbclnt/smbfs_vnops.c
> 1) smbfs_open
> --
>        /*
>         * This thread did the open.
>         * Save our credentials too.
>         */
>        mutex_enter(&np->r_statelock);
>        oldcr = np->r_cred;
>        np->r_cred = cr;
>        crhold(cr);
>        if (oldcr)
>                crfree(oldcr);
>        mutex_exit(&np->r_statelock);
> --
>
> 2) smbfs_rele_fid
> --
>        /*
>         * Other "last close" stuff.
>         */
>        mutex_enter(&np->r_statelock);
>        if (np->n_flag & NATTRCHANGED)
>                smbfs_attrcache_rm_locked(np);
>        oldcr = np->r_cred;
>        np->r_cred = NULL;
>        mutex_exit(&np->r_statelock);
>        if (oldcr != NULL)
>                crfree(oldcr);
> --
>
> File uts/common/fs/smbclnt/smbfs/smbfs_subr2.c
> 1) sn_inactive
> --
>        oldcr = np->r_cred;
>        np->r_cred = NULL;
> ...
>        if (oldcr != NULL)
>                crfree(oldcr);
>
> --
>
> 2) sn_destroy_node
> --
>        ASSERT(np->r_cred == NULL);
> --
>
> As you can see kernel just saves credentials of the last guy who
> opened file on smbfs share to r_cred field of smbfsnode and nothing
> more.
> It doesn't use saved credentials later even for debugging.
>
> What illumos policy says about dead code? Should it be removed or dropped as is?
>
> --
> W.B.R.
> Dan Kruchinin
>
> _______________________________________________
> Developer mailing list
> Developer at lists.illumos.org
> http://lists.illumos.org/m/listinfo/developer
>