readdir() and readdir_r() (XCC)
authorBarret Rhoden <brho@cs.berkeley.edu>
Wed, 18 Aug 2010 05:50:32 +0000 (22:50 -0700)
committerKevin Klues <klueska@cs.berkeley.edu>
Thu, 3 Nov 2011 00:35:52 +0000 (17:35 -0700)
We're currently allowing processes to read() directory FDs, in part
because that is how it was already.  The alternative is to pull in the
glibc stuff related to how Linux does their getdents(), and write that
syscall for ROS.  We'll see how programs really expect readdir() to work
(man 3 readdir, not man 2 readdir).

This patch also brings about per-filetype file_operations, which has
been on the radar/considered for a while.  Right now, they are just used
for having a different read method (and no write method) for
directories.

Rebuild your cross compiler if you want to use the readdir() functions.

kern/include/ros/fs.h
kern/include/vfs.h
kern/src/kfs.c
kern/src/syscall.c
kern/src/vfs.c
tests/file_test.c
tools/compilers/gcc-glibc/glibc-2.11.1-ros/sysdeps/ros/bits/dirent.h
tools/compilers/gcc-glibc/glibc-2.11.1-ros/sysdeps/ros/readdir_r.c [new file with mode: 0644]

index 5abe81f..9865553 100644 (file)
@@ -4,16 +4,18 @@
 #include <sys/types.h>
 #include <timing.h>
 
 #include <sys/types.h>
 #include <timing.h>
 
+/* Keep this 255 to stay in sync with glibc (expects d_name[256]) */
 #define MAX_FILENAME_SZ 255
 /* This will change once we have a decent readdir / getdents syscall, and
  * send the strlen along with the d_name.  The sizes need rechecked too, since
  * they are probably wrong. */
 struct kdirent {
 #define MAX_FILENAME_SZ 255
 /* This will change once we have a decent readdir / getdents syscall, and
  * send the strlen along with the d_name.  The sizes need rechecked too, since
  * they are probably wrong. */
 struct kdirent {
-       ino_t                                   d_ino;       /* inode number */
-       off_t                                   d_off;       /* offset to the next dirent */
-       unsigned short                          d_reclen;    /* length of this record */
-       char                                    d_name[MAX_FILENAME_SZ + 1]; /* filename */
-};
+       __ino64_t                                       d_ino;          /* inode number */
+       __off64_t                                       d_off;          /* offset to the next dirent */
+       unsigned short                          d_reclen;       /* length of this record */
+       unsigned char                           d_type;
+       char                                            d_name[MAX_FILENAME_SZ + 1];    /* filename */
+} __attribute__((aligned(8)));
 
 /* These stat sizes should match the types in stat.h and types.h and the sizes
  * in typesizes in glibc (which we modified slightly).  While glibc has it's own
 
 /* These stat sizes should match the types in stat.h and types.h and the sizes
  * in typesizes in glibc (which we modified slightly).  While glibc has it's own
index 15c8d1e..e9e054e 100644 (file)
@@ -500,6 +500,8 @@ ssize_t generic_file_read(struct file *file, char *buf, size_t count,
                           off_t *offset);
 ssize_t generic_file_write(struct file *file, const char *buf, size_t count,
                            off_t *offset);
                           off_t *offset);
 ssize_t generic_file_write(struct file *file, const char *buf, size_t count,
                            off_t *offset);
+ssize_t generic_dir_read(struct file *file, char *u_buf, size_t count,
+                         off_t *offset);
 struct file *do_file_open(char *path, int flags, int mode);
 int do_symlink(char *path, const char *symname, int mode);
 int do_file_access(char *path, int mode);
 struct file *do_file_open(char *path, int flags, int mode);
 int do_symlink(char *path, const char *symname, int mode);
 int do_file_access(char *path, int mode);
index 1010d42..6c782b8 100644 (file)
@@ -35,7 +35,9 @@ struct page_map_operations kfs_pm_op;
 struct super_operations kfs_s_op;
 struct inode_operations kfs_i_op;
 struct dentry_operations kfs_d_op;
 struct super_operations kfs_s_op;
 struct inode_operations kfs_i_op;
 struct dentry_operations kfs_d_op;
-struct file_operations kfs_f_op;
+struct file_operations kfs_f_op_file;
+struct file_operations kfs_f_op_dir;
+struct file_operations kfs_f_op_sym;
 
 /* TODO: something more better.  Prob something like the vmem cache, for this,
  * pids, etc.  Good enough for now.  This also means we can only have one
 
 /* TODO: something more better.  Prob something like the vmem cache, for this,
  * pids, etc.  Good enough for now.  This also means we can only have one
@@ -146,14 +148,14 @@ int kfs_readpage(struct file *file, struct page *page)
 
 /* creates and initializes a new inode.  generic fields are filled in.  specific
  * fields are filled in in read_inode() based on what's on the disk for a given
 
 /* creates and initializes a new inode.  generic fields are filled in.  specific
  * fields are filled in in read_inode() based on what's on the disk for a given
- * i_no.  i_no is set by the caller.  Note that this means this inode can be for
- * an inode that is already on disk, or it can be used when creating. */
+ * i_no.  i_no and i_fop are set by the caller.  Note that this means this inode
+ * can be for an inode that is already on disk, or it can be used when creating.
+ * The i_fop depends on the type of file (file, directory, symlink, etc). */
 struct inode *kfs_alloc_inode(struct super_block *sb)
 {
        struct inode *inode = kmem_cache_alloc(inode_kcache, 0);
        memset(inode, 0, sizeof(struct inode));
        inode->i_op = &kfs_i_op;
 struct inode *kfs_alloc_inode(struct super_block *sb)
 {
        struct inode *inode = kmem_cache_alloc(inode_kcache, 0);
        memset(inode, 0, sizeof(struct inode));
        inode->i_op = &kfs_i_op;
-       inode->i_fop = &kfs_f_op;
        inode->i_pm.pm_op = &kfs_pm_op;
        inode->i_fs_info = kmem_cache_alloc(kfs_i_kcache, 0);
        TAILQ_INIT(&((struct kfs_i_info*)inode->i_fs_info)->children);
        inode->i_pm.pm_op = &kfs_pm_op;
        inode->i_fs_info = kmem_cache_alloc(kfs_i_kcache, 0);
        TAILQ_INIT(&((struct kfs_i_info*)inode->i_fs_info)->children);
@@ -179,6 +181,7 @@ void kfs_read_inode(struct inode *inode)
        if (inode->i_ino == 1) {
                inode->i_mode = S_IRWXU | S_IRWXG | S_IRWXO;
                inode->i_type = FS_I_DIR;
        if (inode->i_ino == 1) {
                inode->i_mode = S_IRWXU | S_IRWXG | S_IRWXO;
                inode->i_type = FS_I_DIR;
+               inode->i_fop = &kfs_f_op_dir;
                inode->i_nlink = 1;                             /* assuming only one hardlink */
                inode->i_uid = 0;
                inode->i_gid = 0;
                inode->i_nlink = 1;                             /* assuming only one hardlink */
                inode->i_uid = 0;
                inode->i_gid = 0;
@@ -288,6 +291,7 @@ int kfs_create(struct inode *dir, struct dentry *dentry, int mode,
        struct inode *inode = dentry->d_inode;
        kfs_init_inode(dir, dentry);
        inode->i_type = FS_I_FILE;
        struct inode *inode = dentry->d_inode;
        kfs_init_inode(dir, dentry);
        inode->i_type = FS_I_FILE;
+       inode->i_fop = &kfs_f_op_file;
        /* fs_info->filestart is set by the caller, or else when first written (for
         * new files.  it was set to 0 in alloc_inode(). */
        return 0;
        /* fs_info->filestart is set by the caller, or else when first written (for
         * new files.  it was set to 0 in alloc_inode(). */
        return 0;
@@ -366,6 +370,7 @@ int kfs_symlink(struct inode *dir, struct dentry *dentry, const char *symname)
 
        kfs_init_inode(dir, dentry);
        inode->i_type = FS_I_SYMLINK;
 
        kfs_init_inode(dir, dentry);
        inode->i_type = FS_I_SYMLINK;
+       inode->i_fop = &kfs_f_op_sym;
        strncpy(string, symname, len);
        string[len] = '\0';             /* symname should be \0d anyway, but just in case */
        k_i_info->filestart = string;   /* reusing this void* to hold the char* */
        strncpy(string, symname, len);
        string[len] = '\0';             /* symname should be \0d anyway, but just in case */
        k_i_info->filestart = string;   /* reusing this void* to hold the char* */
@@ -381,7 +386,8 @@ int kfs_mkdir(struct inode *dir, struct dentry *dentry, int mode)
        struct inode *inode = dentry->d_inode;
        kref_get(&dentry->d_kref, 1);   /* to pin the dentry in RAM, KFS-style... */
        inode->i_ino = kfs_get_free_ino();
        struct inode *inode = dentry->d_inode;
        kref_get(&dentry->d_kref, 1);   /* to pin the dentry in RAM, KFS-style... */
        inode->i_ino = kfs_get_free_ino();
-       inode->i_type = FS_I_DIR;               /* this might be FS specific in the future */
+       inode->i_type = FS_I_DIR;
+       inode->i_fop = &kfs_f_op_dir;
        /* get ready to have our own kids */
        TAILQ_INIT(&((struct kfs_i_info*)inode->i_fs_info)->children);
        ((struct kfs_i_info*)inode->i_fs_info)->filestart = 0;
        /* get ready to have our own kids */
        TAILQ_INIT(&((struct kfs_i_info*)inode->i_fs_info)->children);
        ((struct kfs_i_info*)inode->i_fs_info)->filestart = 0;
@@ -671,7 +677,41 @@ struct dentry_operations kfs_d_op = {
        kfs_d_iput,
 };
 
        kfs_d_iput,
 };
 
-struct file_operations kfs_f_op = {
+struct file_operations kfs_f_op_file = {
+       kfs_llseek,
+       generic_file_read,
+       generic_file_write,
+       kfs_readdir,
+       kfs_mmap,
+       kfs_open,
+       kfs_flush,
+       kfs_release,
+       kfs_fsync,
+       kfs_poll,
+       kfs_readv,
+       kfs_writev,
+       kfs_sendpage,
+       kfs_check_flags,
+};
+
+struct file_operations kfs_f_op_dir = {
+       kfs_llseek,
+       generic_dir_read,
+       0,
+       kfs_readdir,
+       kfs_mmap,
+       kfs_open,
+       kfs_flush,
+       kfs_release,
+       kfs_fsync,
+       kfs_poll,
+       kfs_readv,
+       kfs_writev,
+       kfs_sendpage,
+       kfs_check_flags,
+};
+
+struct file_operations kfs_f_op_sym = {
        kfs_llseek,
        generic_file_read,
        generic_file_write,
        kfs_llseek,
        generic_file_read,
        generic_file_write,
index c0d2685..0eadc4f 100644 (file)
@@ -784,6 +784,7 @@ static intreg_t sys_read(struct proc *p, int fd, void *buf, int len)
                set_errno(EBADF);
                return -1;
        }
                set_errno(EBADF);
                return -1;
        }
+       assert(file->f_op->read);
        /* TODO: (UMEM) currently, read() handles user memcpy issues, but we
         * probably should user_mem_check and pin the region here, so read doesn't
         * worry about it */
        /* TODO: (UMEM) currently, read() handles user memcpy issues, but we
         * probably should user_mem_check and pin the region here, so read doesn't
         * worry about it */
@@ -809,6 +810,11 @@ static intreg_t sys_write(struct proc *p, int fd, const void *buf, int len)
                set_errno(EBADF);
                return -1;
        }
                set_errno(EBADF);
                return -1;
        }
+       if (!file->f_op->write) {
+               kref_put(&file->f_kref);
+               set_errno(EINVAL);
+               return -1;
+       }
        /* TODO: (UMEM) */
        ret = file->f_op->write(file, buf, len, &file->f_pos);
        kref_put(&file->f_kref);
        /* TODO: (UMEM) */
        ret = file->f_op->write(file, buf, len, &file->f_pos);
        kref_put(&file->f_kref);
index b1a2819..20cf1ac 100644 (file)
@@ -881,6 +881,46 @@ ssize_t generic_file_write(struct file *file, const char *buf, size_t count,
        return count;
 }
 
        return count;
 }
 
+/* Directories usually use this for their read method, which is the way glibc
+ * currently expects us to do a readdir (short of doing linux's getdents).  Will
+ * probably need work, based on whatever real programs want. */
+ssize_t generic_dir_read(struct file *file, char *u_buf, size_t count,
+                         off_t *offset)
+{
+       struct kdirent dir_r = {0}, *dirent = &dir_r;
+       unsigned int num_dirents = count / sizeof(struct kdirent);
+       int retval = 1;
+       size_t amt_copied = 0;
+       char *buf_end = u_buf + count;
+
+       if (!count)
+               return 0;
+       if (*offset % sizeof(struct kdirent)) {
+               printk("[kernel] the f_pos for a directory should be dirent-aligned\n");
+               set_errno(EINVAL);
+               return -1;
+       }
+       /* for now, we need to tell readdir which dirent we want */
+       dirent->d_off = *offset / sizeof(struct kdirent);
+       for (; (u_buf < buf_end) && (retval == 1); u_buf += sizeof(struct kdirent)){
+               /* TODO: UMEM/KFOP (pin the u_buf in the syscall, ditch the local copy,
+                * get rid of this memcpy and reliance on current, etc).  Might be
+                * tricky with the dirent->d_off */
+               retval = file->f_op->readdir(file, dirent);
+               if (retval < 0)
+                       break;
+               if (current) {
+                       memcpy_to_user(current, u_buf, dirent, sizeof(struct dirent));
+               } else {
+                       memcpy(u_buf, dirent, sizeof(struct dirent));
+               }
+               amt_copied += sizeof(struct dirent);
+               dirent->d_off++;
+       }
+       *offset += amt_copied;
+       return amt_copied;
+}
+
 /* Opens the file, using permissions from current for lack of a better option.
  * It will attempt to create the file if it does not exist and O_CREAT is
  * specified.  This will return 0 on failure, and set errno.  TODO: There's some
 /* Opens the file, using permissions from current for lack of a better option.
  * It will attempt to create the file if it does not exist and O_CREAT is
  * specified.  This will return 0 on failure, and set errno.  TODO: There's some
@@ -940,12 +980,6 @@ struct file *do_file_open(char *path, int flags, int mode)
                        set_errno(EEXIST);
                        return 0;
                }
                        set_errno(EEXIST);
                        return 0;
                }
-               if (file_d->d_inode->i_type == FS_I_DIR) {
-                       kref_put(&file_d->d_kref);
-                       path_release(nd);
-                       set_errno(EISDIR);
-                       return 0;
-               }
        }
        /* 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.*/
        }
        /* 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.*/
index 0911272..8f591e8 100644 (file)
@@ -5,6 +5,7 @@
 #include <arch/arch.h>
 #include <unistd.h>
 #include <errno.h>
 #include <arch/arch.h>
 #include <unistd.h>
 #include <errno.h>
+#include <dirent.h>
 
 int main() 
 { 
 
 int main() 
 { 
@@ -62,6 +63,36 @@ int main()
                printf("WARNING! Readlink failed!\n");
        else
                printf("Readlink read %d bytes\n", retval);
                printf("WARNING! Readlink failed!\n");
        else
                printf("Readlink read %d bytes\n", retval);
+       
+       /* Readdir tests: two ways to do it: */
+       DIR *dir = opendir("/dir1/");
+       struct dirent dirent_r, *dirent, *result = 0;
+       #if 0
+       dirent = readdir(dir);
+       printf("Readdir: d_ino %lld, d_off: %lld, d_reclen: %d, d_name: %s\n",
+              dirent->d_ino, dirent->d_off, dirent->d_reclen, dirent->d_name);
+       printf("TAKE TWO:\n-----------\n");
+       dirent = readdir(dir);
+       printf("Readdir: d_ino %lld, d_off: %lld, d_reclen: %d, d_name: %s\n",
+              dirent->d_ino, dirent->d_off, dirent->d_reclen, dirent->d_name);
+       #endif
+
+       retval = readdir_r(dir, &dirent_r, &result);
+       if (retval > 0)
+               printf("WARNING! Readdir_r failed!, retval %d\n", retval);
+       if (!result)
+               printf("End of the directory\n");
+       else
+               printf("Dirent name: %s\n", result->d_name);
+       printf("TAKE TWO:\n-----------\n");
+       memset(&dirent_r, 0, sizeof(struct dirent));
+       retval = readdir_r(dir, &dirent_r, &result);
+       if (retval > 0)
+               printf("WARNING! Readdir_r failed!, retval %d\n", retval);
+       if (!result)
+               printf("End of the directory\n");
+       else
+               printf("Dirent name: %s\n", result->d_name);
 
        breakpoint();
 }
 
        breakpoint();
 }
index 7c3303f..9597566 100644 (file)
@@ -1,13 +1,12 @@
-
+/* Keep this in sync with the kernel */
 struct dirent
 {
 struct dirent
 {
-  unsigned long long d_ino;
-  unsigned long long d_off;
+  __ino64_t          d_ino;
+  __off64_t          d_off;
   unsigned short     d_reclen;
   unsigned char      d_type;
   char               d_name[256];
 } __attribute__((aligned(8)));
 
 #define d_fileno d_ino
   unsigned short     d_reclen;
   unsigned char      d_type;
   char               d_name[256];
 } __attribute__((aligned(8)));
 
 #define d_fileno d_ino
-
 #define dirent64 dirent
 #define dirent64 dirent
diff --git a/tools/compilers/gcc-glibc/glibc-2.11.1-ros/sysdeps/ros/readdir_r.c b/tools/compilers/gcc-glibc/glibc-2.11.1-ros/sysdeps/ros/readdir_r.c
new file mode 100644 (file)
index 0000000..3309c9e
--- /dev/null
@@ -0,0 +1 @@
+#include <sysdeps/unix/readdir_r.c>