From 1c757ecdfea7182203d0d107b096c65567dc1323 Mon Sep 17 00:00:00 2001 From: sBubshait Date: Fri, 15 Nov 2024 13:44:21 +0000 Subject: [PATCH] Update syscall to add more helpful comments for clarity and readability --- src/userprog/syscall.c | 50 +++++++++++++++++++++++++++++++++++++++++- 1 file changed, 49 insertions(+), 1 deletion(-) diff --git a/src/userprog/syscall.c b/src/userprog/syscall.c index 29231f3..11dd5bd 100644 --- a/src/userprog/syscall.c +++ b/src/userprog/syscall.c @@ -142,6 +142,7 @@ syscall_exit (int status) static pid_t syscall_exec (const char *cmd_line) { + /* Validate the user string before executing the process. */ validate_user_string (cmd_line); return process_execute (cmd_line); /* Returns the PID of the new process */ @@ -161,12 +162,15 @@ syscall_wait (pid_t pid) static bool syscall_create (const char *file, unsigned initial_size) { + /* Validate the user string before creating the file. */ validate_user_string (file); + /* Acquire the file system lock to prevent race conditions. */ lock_acquire (&filesys_lock); bool status = filesys_create (file, initial_size); lock_release (&filesys_lock); + /* Return the status of the file creation. */ return status; } @@ -176,12 +180,15 @@ syscall_create (const char *file, unsigned initial_size) static bool syscall_remove (const char *file) { + /* Validate the user string before removing the file. */ validate_user_string (file); + /* Acquire the file system lock to prevent race conditions. */ lock_acquire (&filesys_lock); bool status = filesys_remove (file); lock_release (&filesys_lock); + /* Return the status of the file removal. */ return status; } @@ -192,11 +199,15 @@ syscall_remove (const char *file) static int syscall_open (const char *file) { + /* Validate the user string before opening the file. */ validate_user_string (file); + /* Acquire the file system lock to prevent race conditions. */ lock_acquire (&filesys_lock); struct file *ptr = filesys_open (file); lock_release (&filesys_lock); + + /* If the file could not be opened, return failure. */ if (ptr == NULL) return EXIT_FAILURE; @@ -206,6 +217,8 @@ syscall_open (const char *file) = (struct open_file*) malloc (sizeof (struct open_file)); if (file_info == NULL) { + /* If we could not allocate memory for the file_info struct, close the + file and return failure. */ file_close (ptr); return EXIT_FAILURE; } @@ -227,14 +240,17 @@ syscall_open (const char *file) static int 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); if (file_info == NULL) return EXIT_FAILURE; + /* Acquire the file system lock to prevent any race conditions. */ lock_acquire (&filesys_lock); int bytes = file_length (file_info->file); lock_release (&filesys_lock); + /* Return the number of bytes in the file. */ 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 allowed. */ - if (fd < 0 || fd == STDOUT_FILENO) + if (fd < STDIN_FILENO || fd == STDOUT_FILENO) return EXIT_FAILURE; + /* Validate the user buffer for the provided size before reading. */ validate_user_pointer (buffer, size); if (fd == STDIN_FILENO) @@ -259,18 +276,24 @@ syscall_read (int fd, void *buffer, unsigned size) for (int i = 0; i < size; i++) 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; } else { /* 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); if (file_info == NULL) return EXIT_FAILURE; + /* Acquire the file system lock to prevent race-conditions. */ lock_acquire (&filesys_lock); int bytes_written = file_read (file_info->file, buffer, size); lock_release (&filesys_lock); + + /* Return the number of bytes read. */ return bytes_written; } } @@ -287,25 +310,32 @@ syscall_write (int fd, const void *buffer, unsigned size) if (fd <= 0) return 0; + /* Validate the user buffer for the provided size before writing. */ validate_user_pointer (buffer, size); if (fd == STDOUT_FILENO) { /* Writing to the console. */ putbuf (buffer, size); + + /* In case of console, write is always successful. So return the size for + the number of bytes written. */ return size; } else { /* 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); if (file_info == NULL) return 0; + /* Acquire the file system lock to prevent race conditions. */ lock_acquire (&filesys_lock); int bytes = file_write (file_info->file, buffer, size); lock_release (&filesys_lock); + /* Return the number of bytes written. */ return bytes; } } @@ -317,9 +347,11 @@ syscall_write (int fd, const void *buffer, unsigned size) static void 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); if (file_info != NULL) { + /* File exists: Acquire the file system lock to prevent race conditions. */ lock_acquire (&filesys_lock); file_seek (file_info->file, position); lock_release (&filesys_lock); @@ -332,14 +364,17 @@ syscall_seek (int fd, unsigned position) static unsigned 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); if (file_info == NULL) return 0; + /* Acquire the file system lock to prevent race conditions. */ lock_acquire (&filesys_lock); unsigned pos = file_tell (file_info->file); lock_release (&filesys_lock); + /* Return the current position in the file. */ return pos; } @@ -349,14 +384,21 @@ syscall_tell (int fd) static void 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); 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); + /* Then, close the file, acquiring the file system lock to prevent race + conditions. */ lock_acquire (&filesys_lock); file_close (file_info->file); lock_release (&filesys_lock); + + /* Free the memory allocated for the file_info struct. */ free (file_info); } } @@ -421,11 +463,13 @@ fd_get_file (int fd) static void validate_user_pointer (const void *start, size_t size) { + /* If the size is 0, we do not need to check anything. */ if (size == 0) return; 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)) syscall_exit (EXIT_FAILURE); @@ -442,9 +486,11 @@ validate_user_pointer (const void *start, size_t size) static void validate_user_string (const char *str) { + /* Check if the string pointer is a valid user virtual address. */ if (str == NULL || !is_user_vaddr (str)) syscall_exit (EXIT_FAILURE); + /* Calculate the offset of the string within the (first) page. */ size_t offset = (uintptr_t) str % PGSIZE; /* 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); + /* 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) || pagedir_get_page (thread_current ()->pagedir, page) == NULL) syscall_exit (EXIT_FAILURE);