Update syscall to add more helpful comments for clarity and readability

This commit is contained in:
sBubshait
2024-11-15 13:44:21 +00:00
parent 6a1d10a19b
commit 1c757ecdfe

View File

@@ -142,6 +142,7 @@ syscall_exit (int status)
static pid_t static pid_t
syscall_exec (const char *cmd_line) syscall_exec (const char *cmd_line)
{ {
/* Validate the user string before executing the process. */
validate_user_string (cmd_line); validate_user_string (cmd_line);
return process_execute (cmd_line); /* Returns the PID of the new process */ return process_execute (cmd_line); /* Returns the PID of the new process */
@@ -161,12 +162,15 @@ syscall_wait (pid_t pid)
static bool static bool
syscall_create (const char *file, unsigned initial_size) syscall_create (const char *file, unsigned initial_size)
{ {
/* Validate the user string before creating the file. */
validate_user_string (file); validate_user_string (file);
/* Acquire the file system lock to prevent race conditions. */
lock_acquire (&filesys_lock); lock_acquire (&filesys_lock);
bool status = filesys_create (file, initial_size); bool status = filesys_create (file, initial_size);
lock_release (&filesys_lock); lock_release (&filesys_lock);
/* Return the status of the file creation. */
return status; return status;
} }
@@ -176,12 +180,15 @@ syscall_create (const char *file, unsigned initial_size)
static bool static bool
syscall_remove (const char *file) syscall_remove (const char *file)
{ {
/* Validate the user string before removing the file. */
validate_user_string (file); validate_user_string (file);
/* Acquire the file system lock to prevent race conditions. */
lock_acquire (&filesys_lock); lock_acquire (&filesys_lock);
bool status = filesys_remove (file); bool status = filesys_remove (file);
lock_release (&filesys_lock); lock_release (&filesys_lock);
/* Return the status of the file removal. */
return status; return status;
} }
@@ -192,11 +199,15 @@ syscall_remove (const char *file)
static int static int
syscall_open (const char *file) syscall_open (const char *file)
{ {
/* Validate the user string before opening the file. */
validate_user_string (file); validate_user_string (file);
/* Acquire the file system lock to prevent race conditions. */
lock_acquire (&filesys_lock); lock_acquire (&filesys_lock);
struct file *ptr = filesys_open (file); struct file *ptr = filesys_open (file);
lock_release (&filesys_lock); lock_release (&filesys_lock);
/* If the file could not be opened, return failure. */
if (ptr == NULL) if (ptr == NULL)
return EXIT_FAILURE; return EXIT_FAILURE;
@@ -206,6 +217,8 @@ syscall_open (const char *file)
= (struct open_file*) malloc (sizeof (struct open_file)); = (struct open_file*) malloc (sizeof (struct open_file));
if (file_info == NULL) if (file_info == NULL)
{ {
/* If we could not allocate memory for the file_info struct, close the
file and return failure. */
file_close (ptr); file_close (ptr);
return EXIT_FAILURE; return EXIT_FAILURE;
} }
@@ -227,14 +240,17 @@ syscall_open (const char *file)
static int static int
syscall_filesize (int fd) syscall_filesize (int fd)
{ {
/* Try to get the file from the FD. If it does not exist, return failure. */
struct open_file *file_info = fd_get_file (fd); struct open_file *file_info = fd_get_file (fd);
if (file_info == NULL) if (file_info == NULL)
return EXIT_FAILURE; return EXIT_FAILURE;
/* Acquire the file system lock to prevent any race conditions. */
lock_acquire (&filesys_lock); lock_acquire (&filesys_lock);
int bytes = file_length (file_info->file); int bytes = file_length (file_info->file);
lock_release (&filesys_lock); lock_release (&filesys_lock);
/* Return the number of bytes in the file. */
return bytes; return bytes;
} }
@@ -247,9 +263,10 @@ syscall_read (int fd, void *buffer, unsigned size)
{ {
/* Only console (fd = 0) or other files, not including STDOUT, (fd > 1) are /* Only console (fd = 0) or other files, not including STDOUT, (fd > 1) are
allowed. */ allowed. */
if (fd < 0 || fd == STDOUT_FILENO) if (fd < STDIN_FILENO || fd == STDOUT_FILENO)
return EXIT_FAILURE; return EXIT_FAILURE;
/* Validate the user buffer for the provided size before reading. */
validate_user_pointer (buffer, size); validate_user_pointer (buffer, size);
if (fd == STDIN_FILENO) if (fd == STDIN_FILENO)
@@ -259,18 +276,24 @@ syscall_read (int fd, void *buffer, unsigned size)
for (int i = 0; i < size; i++) for (int i = 0; i < size; i++)
write_buffer[i] = input_getc (); write_buffer[i] = input_getc ();
/* In case of console, read is always (eventually) successful. So return
the size for the number of bytes read. */
return size; return size;
} }
else else
{ {
/* Reading from a file. */ /* Reading from a file. */
/* Find the file from the FD. If it does not exist, return failure. */
struct open_file *file_info = fd_get_file (fd); struct open_file *file_info = fd_get_file (fd);
if (file_info == NULL) if (file_info == NULL)
return EXIT_FAILURE; return EXIT_FAILURE;
/* Acquire the file system lock to prevent race-conditions. */
lock_acquire (&filesys_lock); lock_acquire (&filesys_lock);
int bytes_written = file_read (file_info->file, buffer, size); int bytes_written = file_read (file_info->file, buffer, size);
lock_release (&filesys_lock); lock_release (&filesys_lock);
/* Return the number of bytes read. */
return bytes_written; return bytes_written;
} }
} }
@@ -287,25 +310,32 @@ syscall_write (int fd, const void *buffer, unsigned size)
if (fd <= 0) if (fd <= 0)
return 0; return 0;
/* Validate the user buffer for the provided size before writing. */
validate_user_pointer (buffer, size); validate_user_pointer (buffer, size);
if (fd == STDOUT_FILENO) if (fd == STDOUT_FILENO)
{ {
/* Writing to the console. */ /* Writing to the console. */
putbuf (buffer, size); putbuf (buffer, size);
/* In case of console, write is always successful. So return the size for
the number of bytes written. */
return size; return size;
} }
else else
{ {
/* Writing to a file. */ /* Writing to a file. */
/* Find the file from the FD. If it does not exist, return failure. */
struct open_file *file_info = fd_get_file (fd); struct open_file *file_info = fd_get_file (fd);
if (file_info == NULL) if (file_info == NULL)
return 0; return 0;
/* Acquire the file system lock to prevent race conditions. */
lock_acquire (&filesys_lock); lock_acquire (&filesys_lock);
int bytes = file_write (file_info->file, buffer, size); int bytes = file_write (file_info->file, buffer, size);
lock_release (&filesys_lock); lock_release (&filesys_lock);
/* Return the number of bytes written. */
return bytes; return bytes;
} }
} }
@@ -317,9 +347,11 @@ syscall_write (int fd, const void *buffer, unsigned size)
static void static void
syscall_seek (int fd, unsigned position) syscall_seek (int fd, unsigned position)
{ {
/* Find the file from the FD. If it does not exist, do nothing. */
struct open_file *file_info = fd_get_file (fd); struct open_file *file_info = fd_get_file (fd);
if (file_info != NULL) if (file_info != NULL)
{ {
/* File exists: Acquire the file system lock to prevent race conditions. */
lock_acquire (&filesys_lock); lock_acquire (&filesys_lock);
file_seek (file_info->file, position); file_seek (file_info->file, position);
lock_release (&filesys_lock); lock_release (&filesys_lock);
@@ -332,14 +364,17 @@ syscall_seek (int fd, unsigned position)
static unsigned static unsigned
syscall_tell (int fd) syscall_tell (int fd)
{ {
/* Find the file from the FD. If it does not exist, return 0. */
struct open_file *file_info = fd_get_file (fd); struct open_file *file_info = fd_get_file (fd);
if (file_info == NULL) if (file_info == NULL)
return 0; return 0;
/* Acquire the file system lock to prevent race conditions. */
lock_acquire (&filesys_lock); lock_acquire (&filesys_lock);
unsigned pos = file_tell (file_info->file); unsigned pos = file_tell (file_info->file);
lock_release (&filesys_lock); lock_release (&filesys_lock);
/* Return the current position in the file. */
return pos; return pos;
} }
@@ -349,14 +384,21 @@ syscall_tell (int fd)
static void static void
syscall_close (int fd) syscall_close (int fd)
{ {
/* Find the file from the FD. If it does not exist, do nothing. */
struct open_file *file_info = fd_get_file (fd); struct open_file *file_info = fd_get_file (fd);
if (file_info != NULL) if (file_info != NULL)
{ {
/* File exists */
/* First, remove the file from the hash table of open files. */
hash_delete (&thread_current ()->open_files, &file_info->elem); hash_delete (&thread_current ()->open_files, &file_info->elem);
/* Then, close the file, acquiring the file system lock to prevent race
conditions. */
lock_acquire (&filesys_lock); lock_acquire (&filesys_lock);
file_close (file_info->file); file_close (file_info->file);
lock_release (&filesys_lock); lock_release (&filesys_lock);
/* Free the memory allocated for the file_info struct. */
free (file_info); free (file_info);
} }
} }
@@ -421,11 +463,13 @@ fd_get_file (int fd)
static void static void
validate_user_pointer (const void *start, size_t size) validate_user_pointer (const void *start, size_t size)
{ {
/* If the size is 0, we do not need to check anything. */
if (size == 0) if (size == 0)
return; return;
const void *end = start + size - 1; const void *end = start + size - 1;
/* Check if the start and end pointers are valid user virtual addresses. */
if (start == NULL || !is_user_vaddr (start) || !is_user_vaddr (end)) if (start == NULL || !is_user_vaddr (start) || !is_user_vaddr (end))
syscall_exit (EXIT_FAILURE); syscall_exit (EXIT_FAILURE);
@@ -442,9 +486,11 @@ validate_user_pointer (const void *start, size_t size)
static void static void
validate_user_string (const char *str) validate_user_string (const char *str)
{ {
/* Check if the string pointer is a valid user virtual address. */
if (str == NULL || !is_user_vaddr (str)) if (str == NULL || !is_user_vaddr (str))
syscall_exit (EXIT_FAILURE); syscall_exit (EXIT_FAILURE);
/* Calculate the offset of the string within the (first) page. */
size_t offset = (uintptr_t) str % PGSIZE; size_t offset = (uintptr_t) str % PGSIZE;
/* We move page by page, checking if the page is mapped to physical memory. */ /* We move page by page, checking if the page is mapped to physical memory. */
@@ -452,6 +498,8 @@ validate_user_string (const char *str)
{ {
void *page = pg_round_down (str); void *page = pg_round_down (str);
/* If we reach addresses that are not mapped to physical memory before the
end of the string, the thread is terminated. */
if (!is_user_vaddr(page) || if (!is_user_vaddr(page) ||
pagedir_get_page (thread_current ()->pagedir, page) == NULL) pagedir_get_page (thread_current ()->pagedir, page) == NULL)
syscall_exit (EXIT_FAILURE); syscall_exit (EXIT_FAILURE);