From: Ben Pfaff <blp@cs.stanford.edu>
Date: Fri, 19 May 2006 20:38:50 +0000 (+0000)
Subject: Replace lsdir system call by readdir, isdir system calls,
X-Git-Url: https://pintos-os.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=550e44853cf80d69ae5edf8c2d3b583e8d37dbd8;p=pintos-anon

Replace lsdir system call by readdir, isdir system calls,
which are far less offensive to good taste.
Add dir_get_inode() and dir_readdir() functions.
Remove dir_list(), filesys_list().  The latter was unused.
Update project documentation, solutions, and tests.
Rewrite "ls" example program to use the new interface, and add the
ability to specify a directory name and a "long format" feature.
Add "cd" command to shell.
---

diff --git a/TODO b/TODO
index 0e8047e..3883615 100644
--- a/TODO
+++ b/TODO
@@ -97,8 +97,6 @@ via Godmar Back:
 
     . Other good ideas.
 
-    . opendir/readdir/closedir
-
     . everything needed for getcwd()
 
 To add partition support:
diff --git a/doc/filesys.texi b/doc/filesys.texi
index 16d0fad..8e4dc4f 100644
--- a/doc/filesys.texi
+++ b/doc/filesys.texi
@@ -136,7 +136,7 @@ that a file cannot exceed the size of the disk (minus metadata).  This
 also applies to the root directory file, which should now be allowed
 to expand beyond its initial limit of 16 files.
 
-The user is allowed to seek beyond the current end-of-file (EOF).  The
+User programs are allowed to seek beyond the current end-of-file (EOF).  The
 seek itself does not extend the file.  Writing at a position past EOF
 extends the file to the position being written, and any gap between the
 previous EOF and the start of the write must be filled with zeros.  A
@@ -176,11 +176,19 @@ not an external program.)
 Update the existing system calls so that, anywhere a file name is
 provided by the caller, an absolute or relative path name may used.
 The directory separator character is forward slash (@samp{/}).
+You may support @file{.} and @file{..} for a small amount of extra
+credit.
 
 Update the @code{remove} system call so that it can delete empty
 directories in addition to regular files.  Directories can only be
 deleted if they do not contain any files or subdirectories.
 
+Update the @code{open} system call so that it can also open directories.
+Passing @file{.} as the argument to @code{open} must open the current
+directory, regardless of whether @file{.} and @file{..} are fully
+implemented.  Of the existing system calls, only @code{close} needs to
+accept a file descriptor for a directory.
+
 Implement the following new system calls:
 
 @deftypefn {System Call} bool chdir (const char *@var{dir})
@@ -198,22 +206,39 @@ Fails if @var{dir} already exists or if any directory name in
 @file{/a/b/c} does not.
 @end deftypefn
 
-@deftypefn {System Call} void lsdir (void)
-Prints a list of files in the current directory to @code{stdout}, one
-per line, in no particular order.
+@deftypefn {System Call} bool readdir (int @var{fd}, char *@var{name})
+Reads a directory entry from file descriptor @var{fd}, which must
+represent a directory.  If successful, stores the null-terminated file
+name in @var{name}, which must have room for @code{READDIR_MAX_LEN + 1}
+bytes, and returns true.  If no entries are left in the directory,
+returns false.
+
+@file{.} and @file{..} should not be returned by @code{readdir},
+regardless of whether they are implemented.
+
+If the directory changes while it is open, then it is acceptable for
+some entries not to be read at all or to be read multiple times.
+Otherwise, each directory entry should be read once, in any order.
+
+@code{READDIR_MAX_LEN} is defined in @file{lib/user/syscall.h}.  If your
+file system supports longer file names than the basic file system, you
+should increase this value from the default of 14.
+@end deftypefn
+
+@deftypefn {System Call} bool isdir (int @var{fd})
+Returns true if @var{fd} represents a directory,
+false if it represents an ordinary file.
 @end deftypefn
 
 We have provided @command{ls} and @command{mkdir} user programs, which
-are straightforward once the above syscalls are implemented.
+are straightforward once the above syscalls are implemented.  The
+@command{shell} program implements @command{cd} internally.
 
 The @code{pintos} @option{put} and @option{get} commands should now
 accept full path names, assuming that the directories used in the
 paths have already been created.  This should not require any extra
 effort on your part.
 
-You may support @file{.} and @file{..} for a small amount of extra
-credit.
-
 @node Buffer Cache
 @subsection Buffer Cache
 
@@ -375,6 +400,7 @@ of IDE disk hardware.
 
 @menu
 * Indexed Files FAQ::           
+* Subdirectories FAQ::          
 * Buffer Cache FAQ::            
 @end menu
 
@@ -389,6 +415,22 @@ will have to be smaller than the disk to accommodate the metadata.
 You'll need to consider this when deciding your inode organization.
 @end table
 
+@node Subdirectories FAQ
+@subsection Subdirectories FAQ
+
+@table @b
+@item How should a file name like @samp{//a//b} be interpreted?
+
+Multiple consecutive slashes are equivalent to a single slash, so this
+file name is the same as @samp{/a/b}.
+
+@item How about a file name like @samp{/../x}?
+
+If you don't implement @file{.} and @file{..}, then this is not a
+special case.  If you do, then it is equivalent to @samp{/x}.  That is,
+the root directory is its own parent.
+@end table
+
 @node Buffer Cache FAQ
 @subsection Buffer Cache FAQ
 
diff --git a/solutions/p4.patch b/solutions/p4.patch
index 2342ca9..44c0c2c 100644
--- a/solutions/p4.patch
+++ b/solutions/p4.patch
@@ -722,16 +722,23 @@ diff -u src/filesys/directory.c~ src/filesys/directory.c
    return success;
  }
  
-@@ -205,8 +210,10 @@ dir_list (const struct dir *dir)
+@@ -216,14 +216,17 @@
+ {
    struct dir_entry e;
-   size_t ofs;
-   
+ 
 +  inode_lock (dir->inode);
-   for (ofs = 0; inode_read_at (dir->inode, &e, sizeof e, ofs) == sizeof e;
-        ofs += sizeof e) 
-     if (e.in_use)
-       printf ("%s\n", e.name);
+   while (inode_read_at (dir->inode, &e, sizeof e, dir->pos) == sizeof e) 
+     {
+       dir->pos += sizeof e;
+       if (e.in_use)
+         {
+           strlcpy (name, e.name, NAME_MAX + 1);
++          inode_unlock (dir->inode);
+           return true;
+         } 
+     }
 +  inode_unlock (dir->inode);
+   return false;
  }
 Index: src/filesys/directory.h
 diff -u src/filesys/directory.h~ src/filesys/directory.h
@@ -908,7 +915,7 @@ diff -u src/filesys/filesys.c~ src/filesys/filesys.c
    dir_close (dir);
  
    return success;
-@@ -64,17 +161,18 @@ filesys_create (const char *name, off_t 
+@@ -64,17 +161,22 @@ filesys_create (const char *name, off_t 
     otherwise.
     Fails if no file named NAME exists,
     or if an internal memory allocation fails. */
@@ -917,13 +924,17 @@ diff -u src/filesys/filesys.c~ src/filesys/filesys.c
  filesys_open (const char *name)
  {
 -  struct dir *dir = dir_open_root ();
-+  struct dir *dir;
++  struct dir *dir = NULL;
 +  char basename[NAME_MAX + 1];
    struct inode *inode = NULL;
  
 -  if (dir != NULL)
 -    dir_lookup (dir, name, &inode);
-+  if (resolve_name (name, &dir, basename))
++  if (!strcmp (name, "/"))
++    inode = inode_open (ROOT_DIR_SECTOR);
++  else if (!strcmp (name, "."))
++    inode = inode_reopen (dir_get_inode (thread_current ()->wd));
++  else if (resolve_name (name, &dir, basename))
 +    dir_lookup (dir, basename, &inode);
    dir_close (dir);
  
@@ -932,7 +943,7 @@ diff -u src/filesys/filesys.c~ src/filesys/filesys.c
  }
  
  /* Deletes the file named NAME.
-@@ -84,13 +182,56 @@ filesys_open (const char *name)
+@@ -84,7 +182,11 @@ filesys_open (const char *name)
  bool
  filesys_remove (const char *name) 
  {
@@ -946,9 +957,9 @@ diff -u src/filesys/filesys.c~ src/filesys/filesys.c
 +    success = dir_remove (dir, basename);
    dir_close (dir); 
  
+@@ -91,5 +193,44 @@
    return success;
  }
- 
 +/* Change current directory to NAME.
 +   Return true if successful, false on failure. */
 +bool
@@ -988,28 +999,9 @@ diff -u src/filesys/filesys.c~ src/filesys/filesys.c
 +  return true;
 +}
 +
- /* Prints a list of files in the filesystem to the system
-    console.
-    Returns true if successful, false on failure,
-@@ -98,15 +239,9 @@ filesys_remove (const char *name) 
- bool
- filesys_list (void) 
- {
--  struct dir *dir = dir_open_root ();
--  if (dir != NULL) 
--    {
--      dir_list (dir);
--      dir_close (dir);
--      return true;
--    }
--  else
--    return false;
-+  dir_list (thread_current ()->wd);
-+
-+  return true;
- }
  
  static void must_succeed_function (int, bool) NO_INLINE;
+ #define MUST_SUCCEED(EXPR) must_succeed_function (__LINE__, EXPR)
 @@ -129,8 +264,8 @@ filesys_self_test (void)
      {
        /* Create file and check that it contains zeros
@@ -1067,7 +1059,7 @@ diff -u src/filesys/filesys.h~ src/filesys/filesys.h
  
  /* Sectors of system file inodes. */
  #define FREE_MAP_SECTOR 0       /* Free map file inode sector. */
-@@ -13,8 +14,8 @@ extern struct disk *filesys_disk;
+@@ -13,9 +14,10 @@ extern struct disk *filesys_disk;
  
  void filesys_init (bool format);
  void filesys_done (void);
@@ -1076,8 +1068,10 @@ diff -u src/filesys/filesys.h~ src/filesys/filesys.h
 +bool filesys_create (const char *name, off_t initial_size, enum inode_type);
 +struct inode *filesys_open (const char *name);
  bool filesys_remove (const char *name);
- bool filesys_chdir (const char *name);
- bool filesys_list (void);
++bool filesys_chdir (const char *name);
+ 
+ void filesys_self_test (void);
+ 
 Index: src/filesys/free-map.c
 diff -u src/filesys/free-map.c~ src/filesys/free-map.c
 --- src/filesys/free-map.c	2005-06-18 20:20:48.000000000 -0700
@@ -2661,7 +2655,7 @@ Index: src/userprog/syscall.c
 diff -u src/userprog/syscall.c~ src/userprog/syscall.c
 --- src/userprog/syscall.c	2005-06-18 20:21:21.000000000 -0700
 +++ src/userprog/syscall.c	2006-05-18 21:26:51.000000000 -0700
-@@ -1,20 +1,594 @@
+@@ -1,20 +1,671 @@
  #include "userprog/syscall.h"
  #include <stdio.h>
 +#include <string.h>
@@ -2699,7 +2693,8 @@ diff -u src/userprog/syscall.c~ src/userprog/syscall.c
 +static int sys_munmap (int mapping);
 +static int sys_chdir (const char *udir);
 +static int sys_mkdir (const char *udir);
-+static int sys_lsdir (void);
++static int sys_readdir (int handle, char *name);
++static int sys_isdir (int handle);
 + 
  static void syscall_handler (struct intr_frame *);
 -
@@ -2744,7 +2739,8 @@ diff -u src/userprog/syscall.c~ src/userprog/syscall.c
 +      {1, (syscall_function *) sys_munmap},
 +      {1, (syscall_function *) sys_chdir},
 +      {1, (syscall_function *) sys_mkdir},
-+      {0, (syscall_function *) sys_lsdir},
++      {2, (syscall_function *) sys_readdir},
++      {1, (syscall_function *) sys_isdir},
 +    };
  
 +  const struct syscall *sc;
@@ -2794,6 +2790,32 @@ diff -u src/userprog/syscall.c~ src/userprog/syscall.c
 +    }
 +}
 + 
++/* Copies SIZE bytes from kernel address SRC to user address
++   UDST.
++   Call thread_exit() if any of the user accesses are invalid. */
++static void
++copy_out (void *udst_, const void *src_, size_t size) 
++{
++  uint8_t *udst = udst_;
++  const uint8_t *src = src_;
++
++  while (size > 0) 
++    {
++      size_t chunk_size = PGSIZE - pg_ofs (udst);
++      if (chunk_size > size)
++        chunk_size = size;
++      
++      if (!page_lock (udst, false))
++        thread_exit ();
++      memcpy (udst, src, chunk_size);
++      page_unlock (udst);
++
++      udst += chunk_size;
++      src += chunk_size;
++      size -= chunk_size;
++    }
++}
++ 
 +/* Creates a copy of user string US in kernel memory
 +   and returns it as a page that must be freed with
 +   palloc_free_page().
@@ -2903,6 +2925,7 @@ diff -u src/userprog/syscall.c~ src/userprog/syscall.c
 +  {
 +    struct list_elem elem;      /* List element. */
 +    struct file *file;          /* File. */
++    struct dir *dir;            /* Directory. */
 +    int handle;                 /* File handle. */
 +  };
 + 
@@ -2914,18 +2937,28 @@ diff -u src/userprog/syscall.c~ src/userprog/syscall.c
 +  struct file_descriptor *fd;
 +  int handle = -1;
 + 
-+  fd = malloc (sizeof *fd);
++  fd = calloc (1, sizeof *fd);
 +  if (fd != NULL)
 +    {
-+      fd->file = file_open (filesys_open (kfile));
-+      if (fd->file != NULL)
++      struct inode *inode = filesys_open (kfile);
++      if (inode != NULL)
 +        {
-+          struct thread *cur = thread_current ();
-+          handle = fd->handle = cur->next_handle++;
-+          list_push_front (&cur->fds, &fd->elem);
++          if (inode_get_type (inode) == FILE_INODE)
++            fd->file = file_open (inode);
++          else
++            fd->dir = dir_open (inode);
++          if (fd->file != NULL || fd->dir != NULL)
++            {
++              struct thread *cur = thread_current ();
++              handle = fd->handle = cur->next_handle++;
++              list_push_front (&cur->fds, &fd->elem);
++            }
++          else 
++            {
++              free (fd);
++              inode_close (inode);
++            }
 +        }
-+      else 
-+        free (fd);
 +    }
 +  
 +  palloc_free_page (kfile);
@@ -2953,11 +2986,35 @@ diff -u src/userprog/syscall.c~ src/userprog/syscall.c
 +  thread_exit ();
 +}
 + 
++/* Returns the file descriptor associated with the given handle.
++   Terminates the process if HANDLE is not associated with an
++   open ordinary file. */
++static struct file_descriptor *
++lookup_file_fd (int handle) 
++{
++  struct file_descriptor *fd = lookup_fd (handle);
++  if (fd->file == NULL)
++    thread_exit ();
++  return fd;
++}
++ 
++/* Returns the file descriptor associated with the given handle.
++   Terminates the process if HANDLE is not associated with an
++   open directory. */
++static struct file_descriptor *
++lookup_dir_fd (int handle) 
++{
++  struct file_descriptor *fd = lookup_fd (handle);
++  if (fd->dir == NULL)
++    thread_exit ();
++  return fd;
++}
++
 +/* Filesize system call. */
 +static int
 +sys_filesize (int handle) 
 +{
-+  struct file_descriptor *fd = lookup_fd (handle);
++  struct file_descriptor *fd = lookup_file_fd (handle);
 +  int size;
 + 
 +  size = file_length (fd->file);
@@ -2975,7 +3032,7 @@ diff -u src/userprog/syscall.c~ src/userprog/syscall.c
 +
 +  /* Look up file descriptor. */
 +  if (handle != STDIN_FILENO)
-+    fd = lookup_fd (handle);
++    fd = lookup_file_fd (handle);
 +
 +  while (size > 0) 
 +    {
@@ -3035,7 +3092,7 @@ diff -u src/userprog/syscall.c~ src/userprog/syscall.c
 +
 +  /* Lookup up file descriptor. */
 +  if (handle != STDOUT_FILENO)
-+    fd = lookup_fd (handle);
++    fd = lookup_file_fd (handle);
 +
 +  while (size > 0) 
 +    {
@@ -3086,7 +3143,7 @@ diff -u src/userprog/syscall.c~ src/userprog/syscall.c
 +sys_seek (int handle, unsigned position) 
 +{
 +  if ((off_t) position >= 0)
-+    file_seek (lookup_fd (handle)->file, position);
++    file_seek (lookup_file_fd (handle)->file, position);
 +  return 0;
 +}
 + 
@@ -3094,7 +3151,7 @@ diff -u src/userprog/syscall.c~ src/userprog/syscall.c
 +static int
 +sys_tell (int handle) 
 +{
-+  return file_tell (lookup_fd (handle)->file);
++  return file_tell (lookup_file_fd (handle)->file);
 +}
 + 
 +/* Close system call. */
@@ -3103,6 +3160,7 @@ diff -u src/userprog/syscall.c~ src/userprog/syscall.c
 +{
 +  struct file_descriptor *fd = lookup_fd (handle);
 +  file_close (fd->file);
++  dir_close (fd->dir);
 +  list_remove (&fd->elem);
 +  free (fd);
 +  return 0;
@@ -3157,7 +3215,7 @@ diff -u src/userprog/syscall.c~ src/userprog/syscall.c
 +static int
 +sys_mmap (int handle, void *addr)
 +{
-+  struct file_descriptor *fd = lookup_fd (handle);
++  struct file_descriptor *fd = lookup_file_fd (handle);
 +  struct mapping *m = malloc (sizeof *m);
 +  size_t offset;
 +  off_t length;
@@ -3227,12 +3285,24 @@ diff -u src/userprog/syscall.c~ src/userprog/syscall.c
 +  return ok;
 +}
 +
-+/* Lsdir system call. */
++/* Readdir system call. */
 +static int
-+sys_lsdir (void) 
++sys_readdir (int handle, char *uname)
 +{
-+  dir_list (thread_current ()->wd);
-+  return 0;
++  struct file_descriptor *fd = lookup_dir_fd (handle);
++  char name[NAME_MAX + 1];
++  bool ok = dir_readdir (fd->dir, name);
++  if (ok)
++    copy_out (uname, name, strlen (name) + 1);
++  return ok;
++}
++
++/* Isdir system call. */
++static int
++sys_isdir (int handle)
++{
++  struct file_descriptor *fd = lookup_fd (handle);
++  return fd->dir != NULL;
 +}
 + 
 +/* On thread exit, close all open files and unmap all mappings. */
@@ -3247,6 +3317,7 @@ diff -u src/userprog/syscall.c~ src/userprog/syscall.c
 +      struct file_descriptor *fd = list_entry (e, struct file_descriptor, elem);
 +      next = list_next (e);
 +      file_close (fd->file);
++      dir_close (fd->dir);
 +      free (fd);
 +    }
 +   
diff --git a/src/examples/ls.c b/src/examples/ls.c
index d64d01e..5907a6c 100644
--- a/src/examples/ls.c
+++ b/src/examples/ls.c
@@ -1,12 +1,87 @@
 /* ls.c
   
-   Lists the current directory. */
+   Lists the contents of the directory or directories named on
+   the command line, or of the current directory if none are
+   named.
+
+   By default, only the name of each file is printed.  If "-l" is
+   given as the first argument, the type and size of each file is
+   also printed. */
 
 #include <syscall.h>
+#include <stdio.h>
+#include <string.h>
+
+static void
+list_dir (const char *dir, bool verbose) 
+{
+  int dir_fd = open (dir);
+  if (dir_fd == -1) 
+    {
+      printf ("%s: not found\n", dir);
+      return; 
+    }
+
+  if (isdir (dir_fd))
+    {
+      char name[READDIR_MAX_LEN];
+      printf ("%s:\n", dir);
+      while (readdir (dir_fd, name)) 
+        {
+          printf ("%s", name); 
+          if (verbose) 
+            {
+              char full_name[128];
+              int entry_fd;
+
+              if (strcmp (dir, "."))
+                snprintf (full_name, sizeof full_name, "%s/%s", dir, name);
+              else 
+                {
+                  /* This is a special case for implementations
+                     that don't fully understand . and .. */
+                  strlcpy (full_name, name, sizeof full_name); 
+                }
+              entry_fd = open (full_name);
+
+              printf (": ");
+              if (entry_fd != -1)
+                {
+                  if (isdir (entry_fd))
+                    printf ("directory");
+                  else
+                    printf ("%d-byte file", filesize (entry_fd));
+                }
+              else
+                printf ("open failed");
+              close (entry_fd);
+            }
+          printf ("\n");
+        }
+    }
+  else 
+    printf ("%s: not a directory\n", dir);
+  close (dir_fd); 
+}
 
 int
-main (void) 
+main (int argc, char *argv[]) 
 {
-  lsdir ();
+  bool verbose = false;
+  if (argc > 1 && !strcmp (argv[1], "-l")) 
+    {
+      verbose = true;
+      argv++;
+      argc--;
+    }
+  
+  if (argc <= 1)
+    list_dir (".", verbose);
+  else 
+    {
+      int i;
+      for (i = 1; i < argc; i++)
+        list_dir (argv[i], verbose);
+    }
   return 0;
 }
diff --git a/src/examples/shell.c b/src/examples/shell.c
index d31eb3a..0aaafbb 100644
--- a/src/examples/shell.c
+++ b/src/examples/shell.c
@@ -21,6 +21,11 @@ main (void)
       /* Execute command. */
       if (!strcmp (command, "exit"))
         break;
+      else if (!memcmp (command, "cd ", 3)) 
+        {
+          if (!chdir (command + 3))
+            printf ("\"%s\": chdir failed\n", command + 3);
+        }
       else if (command[0] == '\0') 
         {
           /* Empty command. */
diff --git a/src/filesys/directory.c b/src/filesys/directory.c
index 45bbdae..0d265d5 100644
--- a/src/filesys/directory.c
+++ b/src/filesys/directory.c
@@ -10,6 +10,7 @@
 struct dir 
   {
     struct inode *inode;                /* Backing store. */
+    off_t pos;                          /* Current position. */
   };
 
 /* A single directory entry. */
@@ -37,6 +38,7 @@ dir_open (struct inode *inode)
   if (inode != NULL && dir != NULL)
     {
       dir->inode = inode;
+      dir->pos = 0;
       return dir;
     }
   else
@@ -74,6 +76,13 @@ dir_close (struct dir *dir)
     }
 }
 
+/* Returns the inode encapsulated by DIR. */
+struct inode *
+dir_get_inode (struct dir *dir) 
+{
+  return dir->inode;
+}
+
 /* Searches DIR for a file with the given NAME.
    If successful, returns true, sets *EP to the directory entry
    if EP is non-null, and sets *OFSP to the byte offset of the
@@ -206,15 +215,22 @@ dir_remove (struct dir *dir, const char *name)
   return success;
 }
 
-/* Prints the names of the files in DIR to the system console. */
-void
-dir_list (const struct dir *dir)
+/* Reads the next directory entry in DIR and stores the name in
+   NAME.  Returns true if successful, false if the directory
+   contains no more entries. */
+bool
+dir_readdir (struct dir *dir, char name[NAME_MAX + 1])
 {
   struct dir_entry e;
-  size_t ofs;
-  
-  for (ofs = 0; inode_read_at (dir->inode, &e, sizeof e, ofs) == sizeof e;
-       ofs += sizeof e) 
-    if (e.in_use)
-      printf ("%s\n", e.name);
+
+  while (inode_read_at (dir->inode, &e, sizeof e, dir->pos) == sizeof e) 
+    {
+      dir->pos += sizeof e;
+      if (e.in_use)
+        {
+          strlcpy (name, e.name, NAME_MAX + 1);
+          return true;
+        } 
+    }
+  return false;
 }
diff --git a/src/filesys/directory.h b/src/filesys/directory.h
index cc8692d..f49bd3f 100644
--- a/src/filesys/directory.h
+++ b/src/filesys/directory.h
@@ -17,9 +17,10 @@ struct dir *dir_open (struct inode *);
 struct dir *dir_open_root (void);
 struct dir *dir_reopen (struct dir *);
 void dir_close (struct dir *);
+struct inode *dir_get_inode (struct dir *);
 bool dir_lookup (const struct dir *, const char *name, struct inode **);
 bool dir_add (struct dir *, const char *name, disk_sector_t);
 bool dir_remove (struct dir *, const char *name);
-void dir_list (const struct dir *);
+bool dir_readdir (struct dir *, char name[NAME_MAX + 1]);
 
 #endif /* filesys/directory.h */
diff --git a/src/filesys/filesys.c b/src/filesys/filesys.c
index 39bdff9..5cf1d97 100644
--- a/src/filesys/filesys.c
+++ b/src/filesys/filesys.c
@@ -90,24 +90,6 @@ filesys_remove (const char *name)
 
   return success;
 }
-
-/* Prints a list of files in the filesystem to the system
-   console.
-   Returns true if successful, false on failure,
-   which occurs only if an internal memory allocation fails. */
-bool
-filesys_list (void) 
-{
-  struct dir *dir = dir_open_root ();
-  if (dir != NULL) 
-    {
-      dir_list (dir);
-      dir_close (dir);
-      return true;
-    }
-  else
-    return false;
-}
 
 static void must_succeed_function (int, bool) NO_INLINE;
 #define MUST_SUCCEED(EXPR) must_succeed_function (__LINE__, EXPR)
diff --git a/src/filesys/filesys.h b/src/filesys/filesys.h
index e87cbea..9c30c25 100644
--- a/src/filesys/filesys.h
+++ b/src/filesys/filesys.h
@@ -16,8 +16,6 @@ void filesys_done (void);
 bool filesys_create (const char *name, off_t initial_size);
 struct file *filesys_open (const char *name);
 bool filesys_remove (const char *name);
-bool filesys_chdir (const char *name);
-bool filesys_list (void);
 
 void filesys_self_test (void);
 
diff --git a/src/filesys/fsutil.c b/src/filesys/fsutil.c
index 9c81fb0..14e615f 100644
--- a/src/filesys/fsutil.c
+++ b/src/filesys/fsutil.c
@@ -3,6 +3,7 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
+#include "filesys/directory.h"
 #include "filesys/file.h"
 #include "filesys/filesys.h"
 #include "devices/disk.h"
@@ -14,8 +15,15 @@
 void
 fsutil_ls (char **argv UNUSED) 
 {
+  struct dir *dir;
+  char name[NAME_MAX + 1];
+  
   printf ("Files in the root directory:\n");
-  filesys_list ();
+  dir = dir_open_root ();
+  if (dir == NULL)
+    PANIC ("root dir open failed");
+  while (dir_readdir (dir, name))
+    printf ("%s\n", name);
   printf ("End of listing.\n");
 }
 
diff --git a/src/lib/syscall-nr.h b/src/lib/syscall-nr.h
index ef57c78..eb1e0ee 100644
--- a/src/lib/syscall-nr.h
+++ b/src/lib/syscall-nr.h
@@ -26,7 +26,8 @@ enum
     /* Project 4 only. */
     SYS_CHDIR,                  /* Change the current directory. */
     SYS_MKDIR,                  /* Create a directory. */
-    SYS_LSDIR                   /* List the current directory to stdout. */
+    SYS_READDIR,                /* Reads a directory entry. */
+    SYS_ISDIR                   /* Tests if a fd represents a directory. */
   };
 
 #endif /* lib/syscall-nr.h */
diff --git a/src/lib/user/syscall.c b/src/lib/user/syscall.c
index 2485530..858b77c 100644
--- a/src/lib/user/syscall.c
+++ b/src/lib/user/syscall.c
@@ -165,9 +165,14 @@ mkdir (const char *dir)
   return syscall1 (SYS_MKDIR, dir);
 }
 
-void
-lsdir (void)
+bool
+readdir (int fd, char name[READDIR_MAX_LEN + 1]) 
 {
-  syscall0 (SYS_LSDIR);
+  return syscall2 (SYS_READDIR, fd, name);
 }
 
+bool
+isdir (int fd) 
+{
+  return syscall1 (SYS_ISDIR, fd);
+}
diff --git a/src/lib/user/syscall.h b/src/lib/user/syscall.h
index d03eae1..340d1b1 100644
--- a/src/lib/user/syscall.h
+++ b/src/lib/user/syscall.h
@@ -10,6 +10,8 @@ typedef int pid_t;
 typedef int mapid_t;
 #define MAP_FAILED ((mapid_t) -1)
 
+#define READDIR_MAX_LEN 14
+
 void halt (void) NO_RETURN;
 void exit (int status) NO_RETURN;
 pid_t exec (const char *file);
@@ -27,6 +29,7 @@ mapid_t mmap (int fd, void *addr);
 void munmap (mapid_t);
 bool chdir (const char *dir);
 bool mkdir (const char *dir);
-void lsdir (void);
+bool readdir (int fd, char name[READDIR_MAX_LEN + 1]);
+bool isdir (int fd);
 
 #endif /* lib/user/syscall.h */
diff --git a/src/tests/filesys/extended/dir-lsdir.c b/src/tests/filesys/extended/dir-lsdir.c
index 091fec5..254e595 100644
--- a/src/tests/filesys/extended/dir-lsdir.c
+++ b/src/tests/filesys/extended/dir-lsdir.c
@@ -1,4 +1,4 @@
-/* Just runs lsdir(). */
+/* Lists the contents of a directory using readdir. */
 
 #include <syscall.h>
 #include "tests/lib.h"
@@ -7,5 +7,15 @@
 void
 test_main (void) 
 {
-  lsdir ();
+  int fd;
+  char name[READDIR_MAX_LEN + 1];
+
+  CHECK ((fd = open (".")) > 1, "open .");
+  CHECK (isdir (fd), "isdir(.)");
+
+  while (readdir (fd, name))
+    msg ("readdir: \"%s\"", name);
+
+  msg ("close .");
+  close (fd);
 }
diff --git a/src/tests/filesys/extended/dir-lsdir.ck b/src/tests/filesys/extended/dir-lsdir.ck
index 0ad947c..36a4e52 100644
--- a/src/tests/filesys/extended/dir-lsdir.ck
+++ b/src/tests/filesys/extended/dir-lsdir.ck
@@ -9,24 +9,32 @@ my (@output) = read_text_file ("$test.output");
 common_checks (@output);
 @output = get_core_output (@output);
 
-my ($begin);
-for my $i (0...$#output) {
-    $begin = $i, last if $output[$i] eq '(dir-lsdir) begin';
+must_contain_in_order (\@output,
+		       '(dir-lsdir) open .',
+		       '(dir-lsdir) isdir(.)',
+		       '(dir-lsdir) close .');
+
+sub must_contain_in_order {
+    my ($output, @lines) = @_;
+    my (@line_numbers) = map (find_line ($_, @$output), @lines);
+    for my $i (0...$#lines - 1) {
+	fail "\"$lines[$i]\" follows \"$lines[$i + 1]\" in output\n"
+	  if $line_numbers[$i] > $line_numbers[$i + 1];
+    }
 }
-fail "\"(dir-lsdir) begin\" does not appear in output\n" if !defined $begin;
 
-my ($end);
-for my $i (0...$#output) {
-    $end = $i, last if $output[$i] eq '(dir-lsdir) end';
+sub find_line {
+    my ($line, @output) = @_;
+    for my $i (0...$#output) {
+	return $i if $line eq $output[$i];
+    }
+    fail "\"$line\" does not appear in output\n";
 }
-fail "\"(dir-lsdir) end\" does not appear in output\n" if !defined $end;
-fail "\"begin\" follows \"end\" in output\n" if $begin > $end;
 
 my (%count);
-for my $fn (@output[$begin + 1...$end - 1]) {
-    $fn =~ s/\s+$//;
+for my $fn (map (/readdir: \"([^"]+)\"/, @output)) {
     fail "Unexpected file \"$fn\" in lsdir output\n"
-      unless grep ($_ eq $fn, qw (. .. dir-lsdir));
+      unless grep ($_ eq $fn, qw (dir-lsdir));
     fail "File \"$fn\" listed twice in lsdir output\n"
       if $count{$fn};
     $count{$fn}++;
diff --git a/src/tests/filesys/extended/dir-open.c b/src/tests/filesys/extended/dir-open.c
index c1f6da5..29d18b8 100644
--- a/src/tests/filesys/extended/dir-open.c
+++ b/src/tests/filesys/extended/dir-open.c
@@ -1,6 +1,5 @@
-/* Tries to open a directory.
-   This is allowed to succeed or fail,
-   but if it succeeds then attempts to write to it must fail. */
+/* Opens a directory, then tries to write to it, which must
+   fail. */
 
 #include <syscall.h>
 #include "tests/lib.h"
@@ -10,16 +9,13 @@ void
 test_main (void) 
 {
   int fd;
+  int retval;
   
   CHECK (mkdir ("xyzzy"), "mkdir \"xyzzy\"");
-  msg ("open \"xyzzy\"");
-  fd = open ("xyzzy");
-  if (fd == -1) 
-    msg ("open returned -1 -- ok");
-  else 
-    {
-      int retval = write (fd, "foobar", 6);
-      CHECK (retval == -1, "write \"xyzzy\" (must return -1, actually %d)",
-             retval);
-    }
+  CHECK ((fd = open ("xyzzy")) > 1, "open \"xyzzy\"");
+
+  msg ("write \"xyzzy\"");
+  retval = write (fd, "foobar", 6);
+  CHECK (retval == -1,
+         "write \"xyzzy\" (must return -1, actually %d)", retval);
 }
diff --git a/src/tests/filesys/extended/dir-open.ck b/src/tests/filesys/extended/dir-open.ck
index 09c0083..e16e9a0 100644
--- a/src/tests/filesys/extended/dir-open.ck
+++ b/src/tests/filesys/extended/dir-open.ck
@@ -2,16 +2,18 @@
 use strict;
 use warnings;
 use tests::tests;
-check_expected (IGNORE_EXIT_CODES => 1, [<<'EOF', <<'EOF']);
+check_expected ([<<'EOF', <<'EOF']);
 (dir-open) begin
 (dir-open) mkdir "xyzzy"
 (dir-open) open "xyzzy"
-(dir-open) open returned -1 -- ok
+(dir-open) write "xyzzy"
+(dir-open) write "xyzzy" (must return -1, actually -1)
 (dir-open) end
+dir-open: exit(0)
 EOF
 (dir-open) begin
 (dir-open) mkdir "xyzzy"
 (dir-open) open "xyzzy"
-(dir-open) write "xyzzy" (must return -1, actually -1)
-(dir-open) end
+(dir-open) write "xyzzy"
+dir-open: exit(-1)
 EOF