From a4613d70fb56b93216299f6253698ab0e4bbd46d Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Fri, 19 May 2006 20:38:50 +0000 Subject: [PATCH] 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. --- doc/filesys.texi | 58 +++++++++++++++--- src/examples/ls.c | 81 ++++++++++++++++++++++++- src/examples/shell.c | 5 ++ src/filesys/directory.c | 34 ++++++++--- src/filesys/directory.h | 3 +- src/filesys/filesys.c | 18 ------ src/filesys/filesys.h | 2 - src/filesys/fsutil.c | 10 ++- src/lib/syscall-nr.h | 3 +- src/lib/user/syscall.c | 11 +++- src/lib/user/syscall.h | 5 +- src/tests/filesys/extended/dir-lsdir.c | 14 ++++- src/tests/filesys/extended/dir-lsdir.ck | 32 ++++++---- src/tests/filesys/extended/dir-open.c | 22 +++---- src/tests/filesys/extended/dir-open.ck | 10 +-- 15 files changed, 230 insertions(+), 78 deletions(-) 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/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 +#include +#include + +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 #include #include +#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 #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 #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 -- 2.30.2