From a7f1d519dad92de1133911bc230ba9ffde8ade41 Mon Sep 17 00:00:00 2001 From: sBubshait Date: Fri, 15 Nov 2024 15:09:24 +0000 Subject: [PATCH] Refactor process_wait to add more comments and improve readability --- src/userprog/process.c | 26 +++++++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/src/userprog/process.c b/src/userprog/process.c index 02a5274..1e7b227 100644 --- a/src/userprog/process.c +++ b/src/userprog/process.c @@ -210,7 +210,8 @@ process_init_stack (char *cmd_saveptr, void **esp, char *file_name) so we can safely use strlen here. Filename has already been split from the command line arguments. */ push_to_stack (esp, arg, strlen (arg) + 1); - + + /* Try to allocate memory for the argument pointer. */ struct arg_elem *arg_elem = malloc (sizeof (struct arg_elem)); if (arg_elem == NULL) { @@ -219,9 +220,11 @@ process_init_stack (char *cmd_saveptr, void **esp, char *file_name) return false; } + /* Store the argument pointer in the linked list. */ arg_elem->arg = *esp; list_push_front (&arg_list, &arg_elem->elem); + /* Increment the argument count and get the next argument. */ arg_count++; arg = strtok_r (NULL, " ", &cmd_saveptr); } @@ -242,7 +245,10 @@ process_init_stack (char *cmd_saveptr, void **esp, char *file_name) int overflow_bytes = (PHYS_BASE - *esp) + remaining_size - PGSIZE; if (overflow_bytes > 0) { + /* Calculate the number of pages needed to allocate. */ int pages_needed = DIV_CEIL (overflow_bytes, PGSIZE); + + /* Allocate the pages and map them to the user process. */ for (int i = 1; i < pages_needed + 1; i++) { uint8_t *kpage = palloc_get_page (PAL_USER | PAL_ZERO); @@ -307,11 +313,12 @@ push_to_stack (void **esp, void *data, size_t data_size) * This function will be implemented in task 2. * For now, it does nothing. */ int -process_wait (tid_t child_tid UNUSED) +process_wait (tid_t child_tid) { struct process_result *child_result = NULL; struct list_elem *e; struct thread *cur = thread_current (); + for (e = list_begin (&cur->child_results); e != list_end (&cur->child_results); e = list_next (e)) { @@ -319,27 +326,40 @@ process_wait (tid_t child_tid UNUSED) = list_entry (e, struct process_result, elem); if (result->tid == child_tid) { + /* Found the child process. */ child_result = result; break; } - /* List is ordered, allowing us to break early. */ + + /* List is ordered, allowing us to break early if the child_tid is + greater than the current result's tid. */ else if (result->tid > child_tid) break; } + + /* If the child process was not found, return -1. */ if (child_result == NULL) return -1; + /* Wait for child to die. */ sema_down (&child_result->sema); + /* We need lock release in process_exit, so we need to acquire (and possibly wait) for it here to ensure we don't free the lock memory before it is released in process_exit. */ lock_acquire (&child_result->lock); + /* To prevent waiting for child twice, remove it from the list. No need to use lock since this is the only thread with access to the struct process_result now. */ list_remove (&child_result->elem); + + /* Get the exit status of the child */ int exit_status = child_result->exit_status; + + /* Release the lock */ lock_release (&child_result->lock); + free (child_result); return exit_status; }