Subject: Re: vop_symlink and unused vpp?
To: Bill Studenmund <wrstuden@zembu.com>
From: Assar Westerlund <assar@netbsd.org>
List: tech-kern
Date: 07/17/2001 11:14:06
--=-=-=
Bill Studenmund <wrstuden@zembu.com> writes:
> > a) document that VOP_SYMLINK always gets *a_vpp = NULL in and can
> > choose to set it or not, but that it should be vrefed if != NULL. ufs
> > can always set it, thereby making lfs happy
>
> Sounds the best to me.
These patches follow that line. I'll commit them if there are no
objections.
/assar
--=-=-=
Content-Type: text/x-patch
Content-Disposition: attachment; filename=netbsd-sys2.diff
Index: coda/coda_vnops.c
===================================================================
RCS file: /cvsroot/syssrc/sys/coda/coda_vnops.c,v
retrieving revision 1.25
diff -u -w -r1.25 coda_vnops.c
--- coda/coda_vnops.c 2001/07/03 06:46:52 1.25
+++ coda/coda_vnops.c 2001/07/17 09:08:43
@@ -1655,19 +1655,9 @@
}
/*
- * Okay, now we have to drop locks on dvp. vpp is unlocked, but
- * ref'd. It doesn't matter what happens in either symlink or
- * lookup. Furthermore, there isn't any way for (dvp == *vpp), so
- * we don't bother checking.
+ * Okay, now we have to drop locks on dvp.
*/
/* vput(ap->a_dvp); released earlier */
- if (*ap->a_vpp) {
- VOP_UNLOCK(*ap->a_vpp, 0); /* this line is new!! It is necessary because lookup() calls
- VOP_LOOKUP (coda_lookup) which returns vpp locked. cfs_nb_lookup
- merged with coda_lookup() to become coda_lookup so UNLOCK is
- necessary */
- vrele(*ap->a_vpp);
- }
/*
* Free the name buffer
Index: kern/vfs_syscalls.c
===================================================================
RCS file: /cvsroot/syssrc/sys/kern/vfs_syscalls.c,v
retrieving revision 1.167
diff -u -w -r1.167 vfs_syscalls.c
--- kern/vfs_syscalls.c 2001/06/28 08:04:18 1.167
+++ kern/vfs_syscalls.c 2001/07/17 09:08:43
@@ -1514,6 +1514,8 @@
vattr.va_mode = ACCESSPERMS &~ p->p_cwdi->cwdi_cmask;
VOP_LEASE(nd.ni_dvp, p, p->p_ucred, LEASE_WRITE);
error = VOP_SYMLINK(nd.ni_dvp, &nd.ni_vp, &nd.ni_cnd, &vattr, path);
+ if (error == 0 && nd.ni_vp != NULL)
+ vput(nd.ni_vp);
out:
PNBUF_PUT(path);
return (error);
Index: kern/vnode_if.c
===================================================================
RCS file: /cvsroot/syssrc/sys/kern/vnode_if.c,v
retrieving revision 1.36
diff -u -w -r1.36 vnode_if.c
--- kern/vnode_if.c 2001/05/26 21:34:04 1.36
+++ kern/vnode_if.c 2001/07/17 09:08:43
@@ -868,7 +868,7 @@
const struct vnodeop_desc vop_symlink_desc = {
25,
"vop_symlink",
- 0 | VDESC_VP0_WILLPUT | VDESC_VPP_WILLRELE,
+ 0 | VDESC_VP0_WILLPUT,
vop_symlink_vp_offsets,
VOPARG_OFFSETOF(struct vop_symlink_args, a_vpp),
VDESC_NO_OFFSET,
Index: kern/vnode_if.src
===================================================================
RCS file: /cvsroot/syssrc/sys/kern/vnode_if.src,v
retrieving revision 1.28
diff -u -w -r1.28 vnode_if.src
--- kern/vnode_if.src 2001/05/26 21:33:11 1.28
+++ kern/vnode_if.src 2001/07/17 09:08:44
@@ -338,17 +338,13 @@
#
#% symlink dvp L U U
-#% symlink vpp - U -
+#% symlink vpp - L -
#
#! symlink cnp CREATE, LOCKPARENT
#
-# XXX - note that the return vnode has already been VRELE'ed
-# by the filesystem layer. To use it you must use vget,
-# possibly with a further namei.
-#
vop_symlink {
IN WILLPUT struct vnode *dvp;
- OUT WILLRELE struct vnode **vpp;
+ OUT struct vnode **vpp;
IN struct componentname *cnp;
IN struct vattr *vap;
IN char *target;
Index: miscfs/union/union_vnops.c
===================================================================
RCS file: /cvsroot/syssrc/sys/miscfs/union/union_vnops.c,v
retrieving revision 1.53
diff -u -w -r1.53 union_vnops.c
--- miscfs/union/union_vnops.c 2001/07/04 21:38:00 1.53
+++ miscfs/union/union_vnops.c 2001/07/17 09:08:44
@@ -1534,7 +1534,7 @@
un->un_flags |= UN_KLOCK;
vput(ap->a_dvp);
error = VOP_SYMLINK(dvp, &vp, cnp, ap->a_vap, ap->a_target);
- *ap->a_vpp = NULLVP;
+ *ap->a_vpp = vp;
return (error);
}
Index: nfs/nfs_serv.c
===================================================================
RCS file: /cvsroot/syssrc/sys/nfs/nfs_serv.c,v
retrieving revision 1.59
diff -u -w -r1.59 nfs_serv.c
--- nfs/nfs_serv.c 2000/11/27 08:39:49 1.59
+++ nfs/nfs_serv.c 2001/07/17 09:08:44
@@ -2146,12 +2146,14 @@
vrele(nd.ni_startdir);
else {
if (v3) {
+ if (nd.ni_vp == NULL) {
nd.ni_cnd.cn_nameiop = LOOKUP;
nd.ni_cnd.cn_flags &= ~(LOCKPARENT | SAVESTART | FOLLOW);
nd.ni_cnd.cn_flags |= (NOFOLLOW | LOCKLEAF);
nd.ni_cnd.cn_proc = procp;
nd.ni_cnd.cn_cred = cred;
error = lookup(&nd);
+ }
if (!error) {
memset((caddr_t)fhp, 0, sizeof(nfh));
fhp->fh_fsid = nd.ni_vp->v_mount->mnt_stat.f_fsid;
@@ -2161,8 +2163,11 @@
procp);
vput(nd.ni_vp);
}
- } else
+ } else {
vrele(nd.ni_startdir);
+ if (nd.ni_vp != NULL)
+ vput(nd.ni_vp);
+ }
PNBUF_PUT(nd.ni_cnd.cn_pnbuf);
}
out:
Index: nfs/nfs_vnops.c
===================================================================
RCS file: /cvsroot/syssrc/sys/nfs/nfs_vnops.c,v
retrieving revision 1.134
diff -u -w -r1.134 nfs_vnops.c
--- nfs/nfs_vnops.c 2001/06/07 01:04:40 1.134
+++ nfs/nfs_vnops.c 2001/07/17 09:08:45
@@ -1869,6 +1869,7 @@
struct vnode *newvp = (struct vnode *)0;
const int v3 = NFS_ISV3(dvp);
+ *ap->a_vpp = NULL;
nfsstats.rpccnt[NFSPROC_SYMLINK]++;
slen = strlen(ap->a_target);
nfsm_reqhead(dvp, NFSPROC_SYMLINK, NFSX_FH(v3) + 2*NFSX_UNSIGNED +
Index: ufs/ext2fs/ext2fs_vnops.c
===================================================================
RCS file: /cvsroot/syssrc/sys/ufs/ext2fs/ext2fs_vnops.c,v
retrieving revision 1.33
diff -u -w -r1.33 ext2fs_vnops.c
--- ufs/ext2fs/ext2fs_vnops.c 2001/03/23 21:11:08 1.33
+++ ufs/ext2fs/ext2fs_vnops.c 2001/07/17 09:08:45
@@ -1206,6 +1206,7 @@
error = vn_rdwr(UIO_WRITE, vp, ap->a_target, len, (off_t)0,
UIO_SYSSPACE, IO_NODELOCKED, ap->a_cnp->cn_cred,
(size_t *)0, (struct proc *)0);
+ if (error)
vput(vp);
return (error);
}
Index: ufs/ufs/ufs_vnops.c
===================================================================
RCS file: /cvsroot/syssrc/sys/ufs/ufs/ufs_vnops.c,v
retrieving revision 1.78
diff -u -w -r1.78 ufs_vnops.c
--- ufs/ufs/ufs_vnops.c 2001/05/28 02:50:53 1.78
+++ ufs/ufs/ufs_vnops.c 2001/07/17 09:08:45
@@ -1450,6 +1450,7 @@
error = vn_rdwr(UIO_WRITE, vp, ap->a_target, len, (off_t)0,
UIO_SYSSPACE, IO_NODELOCKED, ap->a_cnp->cn_cred, NULL,
(struct proc *)0);
+ if (error)
vput(vp);
return (error);
}
--=-=-=--