Fixes bug in close_all_files()
[akaros.git] / kern / src / vfs.c
index 739217c..743413b 100644 (file)
@@ -207,7 +207,7 @@ static int follow_symlink(struct nameidata *nd)
 {
        int retval;
        char *symname;
-       if (nd->dentry->d_inode->i_type != FS_I_SYMLINK)
+       if (!S_ISLNK(nd->dentry->d_inode->i_mode))
                return 0;
        if (nd->depth > MAX_SYMLINK_DEPTH)
                return -ELOOP;
@@ -283,9 +283,17 @@ static int link_path_walk(char *path, struct nameidata *nd)
        /* skip all leading /'s */
        while (*link == '/')
                link++;
-       /* if there's nothing left (null terminated), we're done */
-       if (*link == '\0')
+       /* if there's nothing left (null terminated), we're done.  This should only
+        * happen for "/", which if we wanted a PARENT, should fail (there is no
+        * parent). */
+       if (*link == '\0') {
+               if (nd->flags & LOOKUP_PARENT) {
+                       set_errno(ENOENT);
+                       return -1;
+               }
+               /* o/w, we're good */
                return 0;
+       }
        /* iterate through each intermediate link of the path.  in general, nd
         * tracks where we are in the path, as far as dentries go.  once we have the
         * next dentry, we try to update nd based on that dentry.  link is the part
@@ -328,7 +336,7 @@ static int link_path_walk(char *path, struct nameidata *nd)
                 * during the follow_symlink (a symlink could have had a directory at
                 * the end), though it was in the middle of the real path. */
                nd->flags &= ~LOOKUP_DIRECTORY;
-               if (!(nd->dentry->d_inode->i_type & FS_I_DIR))
+               if (!S_ISDIR(nd->dentry->d_inode->i_mode))
                        return -ENOTDIR;
 next_loop:
                /* move through the path string to the next entry */
@@ -345,6 +353,7 @@ next_loop:
         * the last item (link). */
        if (!strcmp(".", link)) {
                if (nd->flags & LOOKUP_PARENT) {
+                       assert(nd->dentry->d_name.name);
                        stash_nd_name(nd, nd->dentry->d_name.name);
                        climb_up(nd);
                }
@@ -353,6 +362,7 @@ next_loop:
        if (!strcmp("..", link)) {
                climb_up(nd);
                if (nd->flags & LOOKUP_PARENT) {
+                       assert(nd->dentry->d_name.name);
                        stash_nd_name(nd, nd->dentry->d_name.name);
                        climb_up(nd);
                }
@@ -363,6 +373,7 @@ next_loop:
        if (!link_dentry) {
                /* if there's no dentry, we are okay if we are looking for the parent */
                if (nd->flags & LOOKUP_PARENT) {
+                       assert(strcmp(link, ""));
                        stash_nd_name(nd, link);
                        return 0;
                } else {
@@ -384,6 +395,7 @@ next_loop:
         * all.  Now we need to climb up to set nd back on the parent, if that's
         * what we wanted */
        if (nd->flags & LOOKUP_PARENT) {
+               assert(nd->dentry->d_name.name);
                stash_nd_name(nd, link_dentry->d_name.name);
                climb_up(nd);
                return 0;
@@ -392,8 +404,7 @@ next_loop:
         * mountpoint still.  FYI: this hasn't been thought through completely. */
        follow_mount(nd);
        /* If we wanted a directory, but didn't get one, error out */
-       if ((nd->flags & LOOKUP_DIRECTORY) &&
-          !(nd->dentry->d_inode->i_type & FS_I_DIR))
+       if ((nd->flags & LOOKUP_DIRECTORY) && !S_ISDIR(nd->dentry->d_inode->i_mode))
                return -ENOTDIR;
        return 0;
 }
@@ -402,10 +413,18 @@ next_loop:
  * initialized for the first call - specifically, we need the intent. 
  * LOOKUP_PARENT and friends go in the flags var, which is not the intent.
  *
+ * If path_lookup wants a PARENT, but hits the top of the FS (root or
+ * otherwise), we want it to error out.  It's still unclear how we want to
+ * handle processes with roots that aren't root, but at the very least, we don't
+ * want to think we have the parent of /, but have / itself.  Due to the way
+ * link_path_walk works, if that happened, we probably don't have a
+ * nd->last.name.  This needs more thought (TODO).
+ *
  * Need to be careful too.  While the path has been copied-in to the kernel,
  * it's still user input.  */
 int path_lookup(char *path, int flags, struct nameidata *nd)
 {
+       int retval;
        printd("Path lookup for %s\n", path);
        /* we allow absolute lookups with no process context */
        if (path[0] == '/') {                   /* absolute lookup */
@@ -425,7 +444,11 @@ int path_lookup(char *path, int flags, struct nameidata *nd)
        kref_get(&nd->dentry->d_kref, 1);
        nd->flags = flags;
        nd->depth = 0;                                  /* used in symlink following */
-       return link_path_walk(path, nd);        
+       retval =  link_path_walk(path, nd);     
+       /* make sure our PARENT lookup worked */
+       if (!retval && (flags & LOOKUP_PARENT))
+               assert(nd->last.name);
+       return retval;
 }
 
 /* Call this after any use of path_lookup when you are done with its results,
@@ -685,7 +708,7 @@ static struct inode *create_inode(struct dentry *dentry, int mode)
        struct inode *inode = get_inode(dentry);
        if (!inode)
                return 0;
-       inode->i_mode = mode;
+       inode->i_mode = mode & S_PMASK; /* note that after this, we have no type */
        inode->i_nlink = 1;
        inode->i_size = 0;
        inode->i_blocks = 0;
@@ -958,19 +981,31 @@ struct file *do_file_open(char *path, int flags, int mode)
        struct nameidata nd_r = {0}, *nd = &nd_r;
        int error;
 
-       /* this isn't quite right, due to the nature of O_CREAT */
-       if (flags & O_CREAT)
-               nd->intent = LOOKUP_CREATE;
-       else
-               nd->intent = LOOKUP_OPEN;
+       /* The file might exist, lets try to just open it right away */
+       nd->intent = LOOKUP_OPEN;
+       error = path_lookup(path, LOOKUP_FOLLOW, nd);
+       if (!error) {
+               /* Still need to make sure we didn't want to O_EXCL create */
+               if ((flags & O_CREAT) && (flags & O_EXCL)) {
+                       set_errno(EEXIST);
+                       goto out_path_only;
+               }
+               file_d = nd->dentry;
+               kref_get(&file_d->d_kref, 1);
+               goto open_the_file;
+       }
+       /* So it didn't already exist, release the path from the previous lookup,
+        * and then we try to create it. */
+       path_release(nd);       
        /* get the parent, following links.  this means you get the parent of the
         * final link (which may not be in 'path' in the first place. */
+       nd->intent = LOOKUP_CREATE;
        error = path_lookup(path, LOOKUP_PARENT | LOOKUP_FOLLOW, nd);
        if (error) {
                set_errno(-error);
                goto out_path_only;
        }
-       /* see if the target is there, handle accordingly */
+       /* see if the target is there (shouldn't be), and handle accordingly */
        file_d = do_lookup(nd->dentry, nd->last.name); 
        if (!file_d) {
                if (!(flags & O_CREAT)) {
@@ -986,12 +1021,16 @@ struct file *do_file_open(char *path, int flags, int mode)
                        goto out_file_d;
                dcache_put(file_d);
        } else {        /* something already exists */
+               /* this can happen due to concurrent access, but needs to be thought
+                * through */
+               panic("File shouldn't be here!");
                if ((flags & O_CREAT) && (flags & O_EXCL)) {
                        /* wanted to create, not open, bail out */
                        set_errno(EEXIST);
                        goto out_file_d;
                }
        }
+open_the_file:
        /* now open the file (freshly created or if it already existed).  At this
         * point, file_d is a refcnt'd dentry, regardless of which branch we took.*/
        if (flags & O_TRUNC)
@@ -1082,7 +1121,7 @@ int do_link(char *old_path, char *new_path)
        if (!old_d)                                     /* errno set by lookup_dentry */
                goto out_link_d;
        /* For now, can only link to files */
-       if (old_d->d_inode->i_type != FS_I_FILE) {
+       if (!S_ISREG(old_d->d_inode->i_mode)) {
                set_errno(EPERM);
                goto out_both_ds;
        }
@@ -1140,7 +1179,7 @@ int do_unlink(char *path)
                goto out_path_only;
        }
        /* Make sure the target is not a directory */
-       if (dentry->d_inode->i_type == FS_I_DIR) {
+       if (S_ISDIR(dentry->d_inode->i_mode)) {
                set_errno(EISDIR);
                goto out_dentry;
        }
@@ -1191,7 +1230,7 @@ int do_chmod(char *path, int mode)
                        retval = -EPERM;
                else
                #endif
-                       nd->dentry->d_inode->i_mode = mode & 0777;
+                       nd->dentry->d_inode->i_mode |= mode & S_PMASK;
        }
        path_release(nd);       
        return retval;
@@ -1261,7 +1300,7 @@ int do_rmdir(char *path)
                set_errno(ENOENT);
                goto out_path_only;
        }
-       if (dentry->d_inode->i_type != FS_I_DIR) {
+       if (!S_ISDIR(dentry->d_inode->i_mode)) {
                set_errno(ENOTDIR);
                goto out_dentry;
        }
@@ -1528,10 +1567,9 @@ struct file *put_file_from_fd(struct files_struct *open_files, int file_desc)
                        assert(file_desc < open_files->max_files);
                        file = open_files->fd[file_desc];
                        open_files->fd[file_desc] = 0;
+                       assert(file);
+                       kref_put(&file->f_kref);
                        CLR_BITMASK_BIT(open_files->open_fds->fds_bits, file_desc);
-                       /* the if case is due to files (stdin) without a *file yet */
-                       if (file)
-                               kref_put(&file->f_kref);
                }
        }
        spin_unlock(&open_files->lock);
@@ -1539,12 +1577,14 @@ struct file *put_file_from_fd(struct files_struct *open_files, int file_desc)
 }
 
 /* Inserts the file in the files_struct, returning the corresponding new file
- * descriptor, or an error code.  We currently grab the first open FD. */
-int insert_file(struct files_struct *open_files, struct file *file)
+ * descriptor, or an error code.  We start looking for open fds from low_fd. */
+int insert_file(struct files_struct *open_files, struct file *file, int low_fd)
 {
        int slot = -1;
+       if ((low_fd < 0) || (low_fd > NR_FILE_DESC_MAX))
+               return -EINVAL;
        spin_lock(&open_files->lock);
-       for (int i = 0; i < open_files->max_fdset; i++) {
+       for (int i = low_fd; i < open_files->max_fdset; i++) {
                if (GET_BITMASK_BIT(open_files->open_fds->fds_bits, i))
                        continue;
                slot = i;
@@ -1574,12 +1614,12 @@ void close_all_files(struct files_struct *open_files, bool cloexec)
                         * have a valid fdset higher than files */
                        assert(i < open_files->max_files);
                        file = open_files->fd[i];
-                       if (cloexec && !(file->f_flags | O_CLOEXEC))
+                       if (cloexec && !(file->f_flags & O_CLOEXEC))
                                continue;
+                       /* Actually close the file */
                        open_files->fd[i] = 0;
-                       /* the if case is due to files (stdin) without a *file yet */
-                       if (file)
-                               kref_put(&file->f_kref);
+                       assert(file);
+                       kref_put(&file->f_kref);
                        CLR_BITMASK_BIT(open_files->open_fds->fds_bits, i);
                }
        }
@@ -1598,12 +1638,13 @@ void clone_files(struct files_struct *src, struct files_struct *dst)
                         * have a valid fdset higher than files */
                        assert(i < src->max_files);
                        file = src->fd[i];
-                       SET_BITMASK_BIT(dst->open_fds->fds_bits, i);
                        assert(i < dst->max_files && dst->fd[i] == 0);
+                       SET_BITMASK_BIT(dst->open_fds->fds_bits, i);
                        dst->fd[i] = file;
-                       /* the if case is due to files (stdin) without a *file yet */
-                       if (file)
-                               kref_get(&file->f_kref, 1);
+                       assert(file);
+                       kref_get(&file->f_kref, 1);
+                       if (i >= dst->next_fd)
+                               dst->next_fd = i + 1;
                }
        }
        spin_unlock(&dst->lock);
@@ -1679,7 +1720,7 @@ static void print_dir(struct dentry *dentry, char *buf, int depth)
        int retval;
        int child_num = 0;
 
-       if (!dentry->d_inode->i_type & FS_I_DIR) {
+       if (!S_ISDIR(dentry->d_inode->i_mode)) {
                warn("Thought this was only directories!!");
                return;
        }
@@ -1703,18 +1744,22 @@ static void print_dir(struct dentry *dentry, char *buf, int depth)
                        if (!child_d)
                                panic("Inconsistent FS, dirent doesn't have a dentry!");
                        /* Recurse for directories, or just print the name for others */
-                       switch (child_d->d_inode->i_type) {
-                               case (FS_I_DIR):
+                       switch (child_d->d_inode->i_mode & __S_IFMT) {
+                               case (__S_IFDIR):
                                        print_dir(child_d, buf, depth + 1);
                                        break;
-                               case (FS_I_FILE):
+                               case (__S_IFREG):
                                        printk("%s%s size(B): %d nlink: %d\n", buf, next.d_name,
                                               child_d->d_inode->i_size, child_d->d_inode->i_nlink);
                                        break;
-                               case (FS_I_SYMLINK):
+                               case (__S_IFLNK):
                                        printk("%s%s -> %s\n", buf, next.d_name,
                                               child_d->d_inode->i_op->readlink(child_d));
                                        break;
+                               case (__S_IFCHR):
+                                       printk("%s%s (char device) nlink: %d\n", buf, next.d_name,
+                                              child_d->d_inode->i_nlink);
+                                       break;
                                default:
                                        warn("Look around you!  Unknown filetype!");
                        }