file: make sys_open() allocate the file structure

Currently sys_open() receives an uninitialized file structure from its
callers, which is awkward as the callers must handle opening a file in
two steps.  It also doesn't fit well with C++'s notion of an object being
always fully initialized and valid.

Fix by making sys_open() allocate and return the file structure.  This also
fixes a window in open() where an fd would point to an uninitialized file
(between fdalloc() and sys_open()).

Signed-off-by: Avi Kivity <avi@cloudius-systems.com>

bsd/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_file.c

@@ -80,14 +80,10 @@ vdev_file_open(vdev_t *vd, uint64_t *psize, uint64_t *max_psize,
 
 	vf = vd->vdev_tsd = kmem_zalloc(sizeof (vdev_file_t), KM_SLEEP);
 
-	error = falloc_noinstall(&fp);
+	error = sys_open(vd->vdev_path, O_RDWR, 0, &fp);
 	if (error)
 		goto out_free_vf;
 
-	error = sys_open(vd->vdev_path, O_RDWR, 0, fp);
-	if (error)
-		goto out_fdrop;
-
 	vf->vf_file = fp;
 
 skip_open:
@@ -96,8 +92,6 @@ skip_open:
 
 	return (0);
 
-out_fdrop:
-	fdrop(fp);
 out_free_vf:
 	kmem_free(vd->vdev_tsd, sizeof (vdev_file_t));
 	vd->vdev_stat.vs_aux = VDEV_AUX_OPEN_FAILED;

fs/vfs/main.cc

@@ -100,10 +100,6 @@ int open(const char *pathname, int flags, ...)
 	int fd, error;
 	int acc;
 
-	error = falloc(&fp, &fd);
-	if (error)
-		goto out_errno;
-
 	acc = 0;
 	switch (flags & O_ACCMODE) {
 	case O_RDONLY:
@@ -119,19 +115,21 @@ int open(const char *pathname, int flags, ...)
 
 	error = task_conv(t, pathname, acc, path);
 	if (error)
-		goto out_fput;
+		goto out_errno;
 
-	error = sys_open(path, flags, mode, fp);
+	error = sys_open(path, flags, mode, &fp);
 	if (error)
-		goto out_fput;
+		goto out_errno;
 
+	error = fdalloc(fp, &fd);
+	if (error)
+		goto out_fput;
 	fdrop(fp);
 	trace_vfs_open_ret(fd);
 	return fd;
 
 out_fput:
 	fdrop(fp);
-	fdclose(fd);
 out_errno:
 	errno = error;
 	trace_vfs_open_err(error);
@@ -695,14 +693,9 @@ int chdir(const char *pathname)
 	if (pathname == NULL)
 		goto out_errno;
 
-	error = falloc_noinstall(&fp);
-	if (error)
-		goto out_errno;
-
 	/* Check if directory exits */
-	error = sys_open(path, O_RDONLY, 0, fp);
+	error = sys_open(path, O_RDONLY, 0, &fp);
 	if (error) {
-		fdrop(fp);
 		goto out_errno;
 	}
 

fs/vfs/vfs.h

@@ -84,7 +84,7 @@ struct task {
 extern const struct vfssw vfssw[];
 
 __BEGIN_DECLS
-int	 sys_open(char *path, int flags, mode_t mode, struct file *fp);
+int	 sys_open(char *path, int flags, mode_t mode, struct file **fp);
 int	 sys_read(struct file *fp, struct iovec *iov, size_t niov,
 		off_t offset, size_t *count);
 int	 sys_write(struct file *fp, struct iovec *iov, size_t niov,

fs/vfs/vfs_syscalls.cc

@@ -55,8 +55,9 @@
 #include "vfs.h"
 
 int
-sys_open(char *path, int flags, mode_t mode, struct file *fp)
+sys_open(char *path, int flags, mode_t mode, struct file **fpp)
 {
+	file *fp;
 	struct dentry *dp, *ddp;
 	struct vnode *vp;
 	char *filename;
@@ -135,16 +136,22 @@ sys_open(char *path, int flags, mode_t mode, struct file *fp)
 			goto out_vn_unlock;
 	}
 
+	error = falloc_noinstall(&fp);
+	if (error)
+		goto out_vn_unlock;
 	finit(fp, flags, DTYPE_VNODE, NULL, &vfs_ops);
 	fp->f_dentry = dp;
 
 	error = VOP_OPEN(vp, fp);
 	if (error)
-		goto out_vn_unlock;
+		goto out_free_fp;
 	vn_unlock(vp);
 
+	*fpp = fp;
 	return 0;
 
+out_free_fp:
+	fdrop(fp);
 out_vn_unlock:
 	vn_unlock(vp);
 out_drele:
@@ -339,13 +346,9 @@ check_dir_empty(char *path)
 
 	DPRINTF(VFSDB_SYSCALL, ("check_dir_empty\n"));
 
-	error = falloc_noinstall(&fp);
-	if (error)
-		return error;
-
-	error = sys_open(path, O_RDONLY, 0, fp);
+	error = sys_open(path, O_RDONLY, 0, &fp);
 	if (error)
-		goto out_fdrop;
+		goto out_error;
 
 	do {
 		error = sys_readdir(fp, &dir);
@@ -357,8 +360,8 @@ check_dir_empty(char *path)
 		error = 0;
 	else if (error == 0)
 		error = EEXIST;
-out_fdrop:
 	fdrop(fp);
+out_error:
 	return error;
 }